conformance: check reason in HTTPRouteInvalidCrossNamespaceParentRef test

[skriss]

Checks for the NotAllowedByListeners reason on
the HTTPRoute's Accepted: false condition in
the HTTPRouteInvalidCrossNamespaceParentRef
test.

Signed-off-by: Steve Kriss krisss@vmware.com

What type of PR is this?
/area conformance

What this PR does / why we need it:
Checks for the NotAllowedByListeners reason on
the HTTPRoute's Accepted: false condition in
the HTTPRouteInvalidCrossNamespaceParentRef
test.

Which issue(s) this PR fixes:

Fixes #1705

Does this PR introduce a user-facing change?:

Checks for the NotAllowedByListeners reason on
the HTTPRoute's Accepted: false condition in
the HTTPRouteInvalidCrossNamespaceParentRef
conformance test.

[shaneutt]

Looks good thank you 👍

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: shaneutt, skriss

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:

• conformance/OWNERS [shaneutt]

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

[robscott]

I think this is OK, but we need to be careful here. As I recall, we don't require implementations to populate conditions on HTTPRoutes making cross-namespace references because some implementations may be running with limited access to namespace(s). We want to allow implementations to run in a restricted access mode, or even a single-namespace mode, and still allow them to be conformant.

With that said, I think it's reasonable to require implementations running conformance tests to have cluster-wide visibility, so I think this LGTM, just need to be very clear that it is valid not to set this condition if you're running in a mode where you don't have visibility to all namespaces.

[robscott]

Maybe add a comment above this that clarifies that:

1. We expect all implementations running conformance tests to have full namespace visibility and when they are running in that mode, this condition needs to be set.
2. It is valid for controllers to run in modes where they have access to a subset of namespace(s). In that case, they are not obligated to populate this condition on routes they can't see.

[skriss]

Ah, I hadn't considered that use case. If you think it's better to not merge this in order to continue to accommodate that use case, that's OK with me for now; otherwise, I'll add the comment you suggest. I imagine at some point we might need to incorporate that into a conformance profile (#1709)?

[robscott]

Thanks @skriss! I think it's good to add this test, just as long as we have a clarifying comment. I think even the implementations that were deployed with limited access by default would likely have a way to watch all namespaces for the sake of conformance tests, just want to make sure it's clear that's not required for every use of Gateway API.

[skriss]

Comment added!

[robscott]

Thanks @skriss!

/lgtm

[dprotaso]

/cherry-pick release-0.6

[dprotaso]

Was just testing to see if the plugin was active

[robscott]

@dprotaso x-ref #1800, probably want to enable some more plugins, agree that cherry-pick would be very helpful.

[shaneutt]

@dprotaso x-ref #1800, probably want to enable some more plugins, agree that cherry-pick would be very helpful.

We talked about this a bit in Slack: https://kubernetes.slack.com/archives/CR0H13KGA/p1678891960116929

The manual cherry-picking is fast and doesn't really bother me, but I too have been thinking about this plugin as well as some others (like triage) that we should be turning on to make things just that little bit more convenient.

[shaneutt]

I've updated #1800 to include this.