gamma: drop provision to omit backendRef

[howardjohn]

What type of PR is this?

/kind gep

What this PR does / why we need it:
IMO, this does not make sense as SHOULD. It should be a MUST or not in the spec.

I have proposed removal for the following reasons:

• Inconsistent with Gateway parent ref (which also means you cannot possibly have a service parent and gateway parent)
• Limits ability to have non-Service parents, as it implies the frontend and backend are the same object

Does this PR introduce a user-facing change?:

NONE

[howardjohn]

@robscott @michaelbeaumont @keithmattix @mikemorris

[michaelbeaumont]

gateway-api/geps/gep-1426.md

Lines 164 to 169 in a8826b8

	### Ports
	By default, a `Service` attachment applies to all ports in the service. Users may want to attach routes to only a *specific* port in a Service. To do so, the `parentRef.port` field should be used.
	If `port` is set, the implementation MUST associate the route only with that port.
	If `port` is not set, the implementation MUST associate the route with all ports defined in the Service.

Doesn't requiring backendRefs make the "all ports" option significantly less useful? Or is there a use case to direct multiple frontend ports to the same backend port?

[howardjohn]

Doesn't requiring backendRefs make the "all ports" option significantly less useful? Or is there a use case to direct multiple frontend ports to the same backend port?

I would say yes, and I think that is the most compelling reason to keep it if we do. But I would note this is a fairly narrow UX optimization; once they start using a different backend (example - canary) then they lose this enhancement. It seems a like a possible foot-gun that to change the backend from svc to svc-v1=50%, svc-v2=50% I need to change the parentRef and backendRef, for example.

[michaelbeaumont]

Doesn't requiring backendRefs make the "all ports" option significantly less useful? Or is there a use case to direct multiple frontend ports to the same backend port?

I would say yes, and I think that is the most compelling reason to keep it if we do. But I would note this is a fairly narrow UX optimization; once they start using a different backend (example - canary) then they lose this enhancement. It seems a like a possible foot-gun that to change the backend from svc to svc-v1=50%, svc-v2=50% I need to change the parentRef and backendRef, for example.

👍 can we drop the optional port for Service parentRefs if we drop optional backendRefs? Then ideally "missing port & kind: Service" in a parentRef would be rejected by the webhook.

But the status quo of optional parentRef port and optional backendRefs works for me.

[robscott]

@howardjohn I think if we do remove this we should clearly note why it was removed and provide very clear guidance for how the situation should (or must) be handled. Removing the "SHOULD" without actually leaving any guidance for how to handle this case could result in more confusion and inconsistency across providers.

[howardjohn]

It should be done gateway-api wide then IMO. It's not gamma specific to have no backendRef (if we merge this, anyways). Which the answer will be a 500 error I think

…

On Wed, May 17, 2023, 2:42 PM Rob Scott ***@***.***> wrote: @howardjohn <https://github.com/howardjohn> I think if we do remove this we should clearly note why it was removed and provide very clear guidance for how the situation should (or must) be handled. Removing the "SHOULD" without actually leaving any guidance for how to handle this case could result in more confusion and inconsistency across providers. — Reply to this email directly, view it on GitHub <#2031 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEYGXIK33ONN7YPEKBJRHLXGVA5LANCNFSM6AAAAAAYFTFTEM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[michaelbeaumont]

LGTM other than line 96

[howardjohn]

Add back removed line and explained this was removed

[robscott]

Thanks @howardjohn! Based on discussion in GAMMA meeting today and approvals above, I think we've got consensus here.

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: howardjohn, keithmattix, michaelbeaumont, robscott

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:

• geps/OWNERS [robscott]

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