Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support SupportedKinds in ListenerStatus #809

Merged
merged 4 commits into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/gateway-api-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ Fields:
Gateway. NKG only supports a single Gateway.
* `listeners`
* `name` - supported.
* `supportedKinds` - not supported.
* `supportedKinds` - supported.
* `attachedRoutes` - supported.
* `conditions` - Supported (Condition/Status/Reason):
* `Accepted/True/Accepted`
Expand Down
3 changes: 3 additions & 0 deletions internal/state/change_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ var _ = Describe("ChangeProcessor", func() {
Routes: map[types.NamespacedName]*graph.Route{
{Namespace: "test", Name: "hr-1"}: expRouteHR1,
},
SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}},
},
"listener-443-1": {
Source: gw1.Spec.Listeners[1],
Expand All @@ -459,6 +460,7 @@ var _ = Describe("ChangeProcessor", func() {
{Namespace: "test", Name: "hr-1"}: expRouteHR1,
},
ResolvedSecret: helpers.GetPointer(client.ObjectKeyFromObject(diffNsTLSSecret)),
SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}},
},
},
Valid: true,
Expand Down Expand Up @@ -544,6 +546,7 @@ var _ = Describe("ChangeProcessor", func() {
Conditions: conditions.NewListenerRefNotPermitted(
"Certificate ref to secret cert-ns/different-ns-tls-secret not permitted by any ReferenceGrant",
),
SupportedKinds: []v1beta1.RouteGroupKind{{Kind: "HTTPRoute"}},
}

expAttachment := &graph.ParentRefAttachmentStatus{
Expand Down
51 changes: 40 additions & 11 deletions internal/state/graph/gateway_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ type Listener struct {
ResolvedSecret *types.NamespacedName
// Conditions holds the conditions of the Listener.
Conditions []conditions.Condition
// SupportedKinds is the list of RouteGroupKinds allowed by the listener.
SupportedKinds []v1beta1.RouteGroupKind
// Valid shows whether the Listener is valid.
// A Listener is considered valid if NKG can generate valid NGINX configuration for it.
Valid bool
Expand Down Expand Up @@ -158,11 +160,14 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener {
}
}

supportedKinds := getListenerSupportedKinds(listener)

if len(conds) > 0 {
return &Listener{
Source: listener,
Conditions: conds,
Valid: false,
Source: listener,
Conditions: conds,
Valid: false,
SupportedKinds: supportedKinds,
}
}

Expand All @@ -171,6 +176,7 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener {
AllowedRouteLabelSelector: allowedRouteSelector,
Routes: make(map[types.NamespacedName]*Route),
Valid: true,
SupportedKinds: supportedKinds,
}

// resolvers might add different conditions to the listener, so we run them all.
Expand Down Expand Up @@ -206,7 +212,21 @@ func validateListenerHostname(listener v1beta1.Listener) []conditions.Condition
return nil
}

func validateListenerAllowedRouteKind(listener v1beta1.Listener) []conditions.Condition {
func getAndValidateListenerSupportedKinds(listener v1beta1.Listener) (
[]conditions.Condition,
[]v1beta1.RouteGroupKind,
) {
if listener.AllowedRoutes == nil || listener.AllowedRoutes.Kinds == nil {
return nil, []v1beta1.RouteGroupKind{
{
Kind: "HTTPRoute",
},
}
}
var conds []conditions.Condition

supportedKinds := make([]v1beta1.RouteGroupKind, 0, len(listener.AllowedRoutes.Kinds))

validHTTPRouteKind := func(kind v1beta1.RouteGroupKind) bool {
if kind.Kind != v1beta1.Kind("HTTPRoute") {
return false
Expand All @@ -219,17 +239,26 @@ func validateListenerAllowedRouteKind(listener v1beta1.Listener) []conditions.Co

switch listener.Protocol {
case v1beta1.HTTPProtocolType, v1beta1.HTTPSProtocolType:
if listener.AllowedRoutes != nil {
for _, kind := range listener.AllowedRoutes.Kinds {
if !validHTTPRouteKind(kind) {
msg := fmt.Sprintf("Unsupported route kind \"%s/%s\"", *kind.Group, kind.Kind)
return conditions.NewListenerInvalidRouteKinds(msg)
}
for _, kind := range listener.AllowedRoutes.Kinds {
if !validHTTPRouteKind(kind) {
msg := fmt.Sprintf("Unsupported route kind \"%s/%s\"", *kind.Group, kind.Kind)
conds = append(conds, conditions.NewListenerInvalidRouteKinds(msg)...)
continue
}
supportedKinds = append(supportedKinds, kind)
}
}
return conds, supportedKinds
}

return nil
func validateListenerAllowedRouteKind(listener v1beta1.Listener) []conditions.Condition {
conds, _ := getAndValidateListenerSupportedKinds(listener)
return conds
}

func getListenerSupportedKinds(listener v1beta1.Listener) []v1beta1.RouteGroupKind {
_, kinds := getAndValidateListenerSupportedKinds(listener)
return kinds
}

func validateListenerLabelSelector(listener v1beta1.Listener) []conditions.Condition {
Expand Down
86 changes: 65 additions & 21 deletions internal/state/graph/gateway_listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,46 +219,91 @@ func TestValidateListenerHostname(t *testing.T) {
}
}

func TestValidateListenerAllowedRouteKind(t *testing.T) {
func TestGetAndValidateListenerSupportedKinds(t *testing.T) {
HTTPRouteGroupKind := []v1beta1.RouteGroupKind{
{
Kind: "HTTPRoute",
Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName),
},
}
TCPRouteGroupKind := []v1beta1.RouteGroupKind{
{
Kind: "TCPRoute",
Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName),
},
}
tests := []struct {
protocol v1beta1.ProtocolType
kind v1beta1.Kind
group v1beta1.Group
name string
kind []v1beta1.RouteGroupKind
expected []v1beta1.RouteGroupKind
expectErr bool
}{
{
protocol: v1beta1.TCPProtocolType,
expectErr: false,
name: "unsupported protocol is ignored",
kind: TCPRouteGroupKind,
expected: []v1beta1.RouteGroupKind{},
},
{
protocol: v1beta1.HTTPProtocolType,
group: "bad-group",
kind: "HTTPRoute",
protocol: v1beta1.HTTPProtocolType,
kind: []v1beta1.RouteGroupKind{
{
Kind: "HTTPRoute",
Group: helpers.GetPointer[v1beta1.Group]("bad-group"),
},
},
expectErr: true,
name: "invalid group",
expected: []v1beta1.RouteGroupKind{},
},
{
protocol: v1beta1.HTTPProtocolType,
group: v1beta1.GroupName,
kind: "TCPRoute",
kind: TCPRouteGroupKind,
expectErr: true,
name: "invalid kind",
expected: []v1beta1.RouteGroupKind{},
},
{
protocol: v1beta1.HTTPProtocolType,
group: v1beta1.GroupName,
kind: "HTTPRoute",
kind: HTTPRouteGroupKind,
expectErr: false,
name: "valid HTTP",
expected: HTTPRouteGroupKind,
},
{
protocol: v1beta1.HTTPSProtocolType,
group: v1beta1.GroupName,
kind: "HTTPRoute",
kind: HTTPRouteGroupKind,
expectErr: false,
name: "valid HTTPS",
expected: HTTPRouteGroupKind,
},
{
protocol: v1beta1.HTTPSProtocolType,
expectErr: false,
name: "valid HTTPS no kind specified",
expected: []v1beta1.RouteGroupKind{
{
Kind: "HTTPRoute",
},
},
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved
},
{
protocol: v1beta1.HTTPProtocolType,
kind: []v1beta1.RouteGroupKind{
{
Kind: "HTTPRoute",
Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName),
},
{
Kind: "bad-kind",
Group: helpers.GetPointer[v1beta1.Group](v1beta1.GroupName),
},
},
expectErr: true,
name: "valid and invalid kinds",
expected: HTTPRouteGroupKind,
},
}

Expand All @@ -268,17 +313,16 @@ func TestValidateListenerAllowedRouteKind(t *testing.T) {

listener := v1beta1.Listener{
Protocol: test.protocol,
AllowedRoutes: &v1beta1.AllowedRoutes{
Kinds: []v1beta1.RouteGroupKind{
{
Kind: test.kind,
Group: &test.group,
},
},
},
}

conds := validateListenerAllowedRouteKind(listener)
if test.kind != nil {
listener.AllowedRoutes = &v1beta1.AllowedRoutes{
Kinds: test.kind,
}
}

conds, kinds := getAndValidateListenerSupportedKinds(listener)
g.Expect(helpers.Diff(test.expected, kinds)).To(BeEmpty())
if test.expectErr {
g.Expect(conds).ToNot(BeEmpty())
} else {
Expand Down
Loading