adding support for more than one mirror backends + adding clarificati…

[LiorLieberman]

…on on mulitple same type filters within the same rule

What type of PR is this?

/kind feature

What this PR does / why we need it:
This permits multiple HTTPRouteFilterRequestMirror filters within the same rule + improve documentation on multiple, same type filters.

There is no reason to forbid multiple mirrors and we can also add a conformance test for it

It was also requested in istio/api#2805.

Which issue(s) this PR fixes:

Fixes #2109

Does this PR introduce a user-facing change?:

enable support for multiple HTTPRouteFilterRequestMirror filters in the same HTTP Rule

/assign @robscott
/cc @robscott
/cc @howardjohn

[LiorLieberman]

/retest

[robscott]

Thanks @LiorLieberman! Do you mind creating a follow up issue to add a conformance test + supported feature to cover this use case?

[LiorLieberman]

Thanks for reviewing @robscott.

Done - #2210

[LiorLieberman]

/retest

[LiorLieberman]

Is there an option for PR authors to set merge type squash?

[robscott]

Is there an option for PR authors to set merge type squash?

I think anyone should be able to do this:

/label tide/merge-method-squash

[robscott]

Thanks @LiorLieberman! This LGTM, but since this is close to the territory of an API change I'd like @shaneutt or @youngnick to approve as well.

/lgtm

[shaneutt]

Let's double check with some other community members:

/cc @sunjayBhatia @arkodg @mlavacca @michaelbeaumont @Xunzhuo

[sunjayBhatia]

👍🏽

[youngnick]

This generally looks good, but I think that we should definitely have a conformance test, with a feature name, for multiple mirror backends (RouteMultipleMirrorBackends maybe?).

Larger issue, I think that we may need to go through for each Extended field and add the feature name at some point, to make the link between conformance tests and field definitions more clear.

[LiorLieberman]

Thanks @youngnick. Will definitely follow up with a conformance test and supported feature for it. There is already an opened issue.

Once this is merged, @howardjohn will approve istio/api#2805 and we will have it implemented in one implementation so we can check the conformance test validity as well.

Since we have a consensus, can we merge it ?

[youngnick]

Thanks Lior, that sounds great. I'm happy for this to go ahead now.

[youngnick]

/lgtm

[Xunzhuo]

/lgtm, this looks great : )

[shaneutt]

/approve
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LiorLieberman, mlavacca, shaneutt, sunjayBhatia, Xunzhuo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [shaneutt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment