ARTEMIS-4924 Proper handling of invalid messages in SNF queues

[gaohoward]

No description provided.

[gtully]

I think we already have a way to prevent sending with rbac if that is the problem.
But we may just need to make use of a dlq for invalid messages in this case. This cause seems like a good match for a dlq as it blocks the head.

[gaohoward]

@gtully @jbertram I've upated the PR. Now when a cluster connection is activated, a Role is added so that when security is enabled any client will be denied of sending messages to store and forward queues. When security is disabled, or user overrides the role in security settings, messages are sent to snf queue will be moved to DLQ when messages are to be added to snf queue.

[clebertsuconic]

I'm running the whole test suite now. if it's good it LGTM.

@gtully let me know what you think... I will post the results for the test suite here. We should probably merge this before the release if it's all good.

[gaohoward]

I'm running the whole test suite now. if it's good it LGTM.

@gtully let me know what you think... I will post the results for the test suite here. We should probably merge this before the release if it's all good.

Thanks @clebertsuconic

[gaohoward]

@gtully PR updated. I added a bit documentation.

[gtully]

This looks good to me. Adding the detail property is a nice addition to help trouble shooting.

[gtully]

I think the commit message needs to be updated to better reflect what it is now doing.

[gaohoward]

@gtully I've updated the commit messages as well as the jira title. Thanks!

[gtully]

The root problem here is an invalid message at the head of the snf queue.
it is good that we can identify it as invalid and that we remove it, and moving it to a dlq makes sense. where there is an option to drop it by not configuring a dlq for the snf.
But the question of what DLQ to use. If for example the messages in a snf queue exipre, I would expect them to go to the expiryQueue of the original target, the snf queue should not have an expiryQueue of its own, otherwise it just gets in the way of processing expired messages.
Is it true that in the case of no queue routing info, we don't know what the original queue is?

For the test, where there is a send to the snfq, that seems odd, really that should just be denied with security. And in that case, the target queue is the snf, so it brings in the snf dlq back into play, which seems wrong for the same reason that a snf expiry queue is wrong!
If we can remove the direct sent to snfq scenario and can find the original message target, then I think we should send to the original target dlq in this case.
if not, maybe this is the best we can do, but is it worth the complexity compared to just rejecting or dropping the invalid message?