aws_ec2 inventory: add includes_filters and excludes_filters

[goneri]

Expose a new include_filters and exclude_filters configuration keys that gives the user the ability to compose the final inventory list with several queries.

Closes: #248
Closes: #182

[goneri]

Feedback on IRC (Cc: @flowerysong @tadeboro).

[15:06] <goneri> Feedback on https://github.com/ansible-collections/amazon.aws/pull/328 welcome.
[15:37] <flowerysong> goneri: I still think the correct interface for this is for filters: to accept a list, as discussed in https://github.com/ansible/ansible/pull/58132
[15:38] <flowerysong> goneri: Something like https://github.com/flowerysong/ansible-amazon.aws/commit/6546a299
[15:41] <tadeboro> I must agree with flowerysong here. Introducting yet another option that just adds hosts to existing filter result feels awkward.
[16:25] <goneri> I would also add an excludes_entries_matching for this one https://github.com/ansible-collections/amazon.aws/issues/182
[16:26] <goneri> in this case the two options would make sense. I don't really like when the behaviour of an option changes because of the type.
[16:26] <goneri> This leads to confusion.

[flowerysong]

Having multiple options that do the same thing leads to even more confusion. If you really think that it's confusing, you can deprecate the old way of passing filters and require that the user specify a list.

[goneri]

[16:30] <tadeboro> For ServiceNow, we implemented https://github.com/ansible-collections/servicenow.itsm/blob/main/docs/servicenow.itsm.servicenow.itsm.now_inventory.rst#examples
[16:30] <goneri> I updated https://github.com/ansible-collections/amazon.aws/pull/328 with your comments
[16:30] <goneri> I guess it's better to continue there.
[16:30] <flowerysong> It doesn't change the behaviour, it just automatically wraps the dict in a list (similar to other options that accept lists.)
[16:30] <tadeboro> We allow users to specify the inclusion and exclusion criteria.
[16:31] <goneri> flowerysong: in one case we've got a OR and another one a AND.
[16:33] <goneri> tadeboro: interesting. I see the includes/excludes keys as a similarity, am I right?.
[16:34] <tadeboro> If you want to also introduce an exclusion criteria, then deprecating the filter and replacing it with something else maybe makes the most sense? Something like `include_filters` and `exclude_filters` maybe?
[16:35] <tadeboro> In ServiceNow's case, we structured things a bit dfferently because we combine inclusions and exclusion into a single query and then just partition the returned hosts.
[16:35] <goneri> Yes! I agree, includes_entries_matching is pretty lame :-)
[16:36] <goneri> Yes, I understand. With AWS, we need to do one request per filter, it's a bit more complicated.
[16:37] <goneri> I suggest to refresh my PR with your name suggestion: include_filters and exclude_filters. Would this work for you?
[16:38] <tadeboro> It would be a bit strange if I sad no after I suggested the names ;)
[16:38] <goneri> Great, I will push a PR tomorrow. Thanks flowerysong and tadeboro.

[goneri]

Having multiple options that do the same thing leads to even more confusion. If you really think that it's confusing, you can deprecate the old way of passing filters and require that the user specify a list.

I don't think we need to deprecate the current filters and force the users to adjust their configurations. These two news include_filters and exclude_filters would only accept list.

[tremble]

It looks like the template and playbook are missing.

I'm not sure why, but it looks like the integration tests didn't fire properly

[goneri]

🤔 indeed.

[tremble]

Mostly looks good, primarily nit-picking at this point.

[jillr]

Thanks very much @goneri, this looks good. Can you please work with the Tower team to schedule this with QE?