Skip to content

Commit

Permalink
feat: attach translation failure events to unsupported objects when E…
Browse files Browse the repository at this point in the history
…xpressionRoutes enabled (#4022)

* feat: attach translation failure events to unsupported objects when ExpressionRoutes enabled

* add CHANGELOG

* add unit tests for translation failures
  • Loading branch information
randmonkey authored May 18, 2023
1 parent ed7c864 commit 6ce0245
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 0 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ Adding a new version? You'll need three changes:
during the start-up phase. From now on, they will be dynamically started in runtime
once their installation is detected, making restarting the process unnecessary.
[#3996](https:/Kong/kubernetes-ingress-controller/pull/3996)
- Disable translation of unspported kubernetes objects when translating to
expression based routes is enabled (`ExpressionRoutes` feature enabled AND
kong using router flavor `expressions`), and generate a translation failure
event attached to each of the unsupported objects.
[#4022](https:/Kong/kubernetes-ingress-controller/pull/4022)

### Changed

Expand Down
12 changes: 12 additions & 0 deletions internal/dataplane/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,18 @@ func (p *Parser) getCerts(secretsToSNIs SecretNameToSNIs) []certWrapper {
return certs
}

func (p *Parser) registerResourceFailureNotSupportedForExpressionRoutes(obj client.Object) {
if p.featureFlags.ExpressionRoutes {
gvk := obj.GetObjectKind().GroupVersionKind()
p.failuresCollector.PushResourceFailure(
fmt.Sprintf(
"resource kind %s/%s.%s not supported when expression routes enabled",
gvk.Group, gvk.Version, gvk.Kind,
), obj,
)
}
}

func mergeCerts(log logrus.FieldLogger, certLists ...[]certWrapper) []kongstate.Certificate {
snisSeen := make(map[string]string)
certsSeen := make(map[string]certWrapper)
Expand Down
199 changes: 199 additions & 0 deletions internal/dataplane/parser/translate_failures_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
package parser

import (
"strings"
"testing"

"github.com/samber/lo"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
knative "knative.dev/networking/pkg/apis/networking/v1alpha1"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/failures"
"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v2/pkg/apis/configuration/v1beta1"
)

// This file contains unit test functions to test translation failures genreated by parser.

func TestTranslationFailureUnsupportedObjectsExpressionRoutes(t *testing.T) {
testCases := []struct {
name string
objects store.FakeObjects
causingObjects []client.Object
}{
{
name: "knative.Ingresses are not supported",
objects: store.FakeObjects{
KnativeIngresses: []*knative.Ingress{
{
TypeMeta: metav1.TypeMeta{
Kind: "Ingress",
APIVersion: knative.SchemeGroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "knative-ing-1",
Namespace: "default",
Annotations: map[string]string{
annotations.KnativeIngressClassKey: annotations.DefaultIngressClass,
},
},
Spec: knative.IngressSpec{},
},
},
},
causingObjects: []client.Object{
&knative.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "knative-ing-1",
Namespace: "default",
},
},
},
},
{
name: "TCPIngresses and UDPIngresses are not supported",
objects: store.FakeObjects{
TCPIngresses: []*kongv1beta1.TCPIngress{
{
TypeMeta: metav1.TypeMeta{
Kind: "TCPIngress",
APIVersion: kongv1beta1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "tcpingress-1",
Namespace: "default",
Annotations: map[string]string{
annotations.IngressClassKey: annotations.DefaultIngressClass,
},
},
},
},
UDPIngresses: []*kongv1beta1.UDPIngress{
{
TypeMeta: metav1.TypeMeta{
Kind: "UDPIngress",
APIVersion: kongv1beta1.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "udpingress-1",
Namespace: "default",
Annotations: map[string]string{
annotations.IngressClassKey: annotations.DefaultIngressClass,
},
},
},
},
},
causingObjects: []client.Object{
&kongv1beta1.TCPIngress{
ObjectMeta: metav1.ObjectMeta{
Name: "tcpingress-1",
Namespace: "default",
},
},
&kongv1beta1.UDPIngress{
ObjectMeta: metav1.ObjectMeta{
Name: "udpingress-1",
Namespace: "default",
},
},
},
},
{
name: "TCPRoutes, UDPRoutes and TLSRoutes in gateway APIs are not supported",
objects: store.FakeObjects{
TCPRoutes: []*gatewayv1alpha2.TCPRoute{
{
TypeMeta: metav1.TypeMeta{
Kind: "TCPRoute",
APIVersion: gatewayv1alpha2.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "tcproute-1",
Namespace: "default",
},
},
},
UDPRoutes: []*gatewayv1alpha2.UDPRoute{
{
TypeMeta: metav1.TypeMeta{
Kind: "UDPRoute",
APIVersion: gatewayv1alpha2.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "udproute-1",
Namespace: "default",
},
},
},
TLSRoutes: []*gatewayv1alpha2.TLSRoute{
{
TypeMeta: metav1.TypeMeta{
Kind: "TLSRoute",
APIVersion: gatewayv1alpha2.GroupVersion.String(),
},
ObjectMeta: metav1.ObjectMeta{
Name: "tlsroute-1",
Namespace: "default",
},
},
},
},
causingObjects: []client.Object{
&gatewayv1alpha2.TCPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "tcproute-1",
Namespace: "default",
},
},
&gatewayv1alpha2.UDPRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "udproute-1",
Namespace: "default",
},
},
&gatewayv1alpha2.TLSRoute{
ObjectMeta: metav1.ObjectMeta{
Name: "tlsroute-1",
Namespace: "default",
},
},
},
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
storer, err := store.NewFakeStore(tc.objects)
require.NoError(t, err)

parser := mustNewParser(t, storer)
parser.featureFlags.ExpressionRoutes = true

result := parser.BuildKongConfig()
t.Log(result.TranslationFailures)
for _, object := range tc.causingObjects {
require.True(t, lo.ContainsBy(result.TranslationFailures, func(f failures.ResourceFailure) bool {
msg := f.Message()
if !strings.Contains(msg, "not supported when expression routes enabled") {
return false
}

causingObjects := f.CausingObjects()
if len(causingObjects) != 1 {
return false
}
causingObject := causingObjects[0]
return object.GetNamespace() == causingObject.GetNamespace() &&
object.GetName() == causingObject.GetName()
}))
}
})

}
}
5 changes: 5 additions & 0 deletions internal/dataplane/parser/translate_knative.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ func (p *Parser) ingressRulesFromKnativeIngress() ingressRules {
secretToSNIs := newSecretNameToSNIs()

for _, ingress := range ingressList {
if p.featureFlags.ExpressionRoutes {
p.registerResourceFailureNotSupportedForExpressionRoutes(ingress)
continue
}

regexPrefix := translators.ControllerPathRegexPrefix
if prefix, ok := ingress.ObjectMeta.Annotations[annotations.AnnotationPrefix+annotations.RegexPrefixKey]; ok {
regexPrefix = prefix
Expand Down
10 changes: 10 additions & 0 deletions internal/dataplane/parser/translate_kong_l4.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ func (p *Parser) ingressRulesFromTCPIngressV1beta1() ingressRules {
})

for _, ingress := range ingressList {
if p.featureFlags.ExpressionRoutes {
p.registerResourceFailureNotSupportedForExpressionRoutes(ingress)
continue
}

ingressSpec := ingress.Spec

result.SecretNameToSNIs.addFromIngressV1TLS(tcpIngressToNetworkingTLS(ingressSpec.TLS), ingress)
Expand Down Expand Up @@ -101,6 +106,11 @@ func (p *Parser) ingressRulesFromUDPIngressV1beta1() ingressRules {
})

for _, ingress := range ingressList {
if p.featureFlags.ExpressionRoutes {
p.registerResourceFailureNotSupportedForExpressionRoutes(ingress)
continue
}

ingressSpec := ingress.Spec

var objectSuccessfullyParsed bool
Expand Down
5 changes: 5 additions & 0 deletions internal/dataplane/parser/translate_tcproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ func (p *Parser) ingressRulesFromTCPRoutes() ingressRules {

var errs []error
for _, tcproute := range tcpRouteList {
if p.featureFlags.ExpressionRoutes {
p.registerResourceFailureNotSupportedForExpressionRoutes(tcproute)
continue
}

if err := p.ingressRulesFromTCPRoute(&result, tcproute); err != nil {
err = fmt.Errorf("TCPRoute %s/%s can't be routed: %w", tcproute.Namespace, tcproute.Name, err)
errs = append(errs, err)
Expand Down
5 changes: 5 additions & 0 deletions internal/dataplane/parser/translate_tlsroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ func (p *Parser) ingressRulesFromTLSRoutes() ingressRules {

var errs []error
for _, tlsroute := range tlsRouteList {
if p.featureFlags.ExpressionRoutes {
p.registerResourceFailureNotSupportedForExpressionRoutes(tlsroute)
continue
}

if err := p.ingressRulesFromTLSRoute(&result, tlsroute); err != nil {
err = fmt.Errorf("TLSRoute %s/%s can't be routed: %w", tlsroute.Namespace, tlsroute.Name, err)
errs = append(errs, err)
Expand Down
5 changes: 5 additions & 0 deletions internal/dataplane/parser/translate_udproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ func (p *Parser) ingressRulesFromUDPRoutes() ingressRules {

var errs []error
for _, udproute := range udpRouteList {
if p.featureFlags.ExpressionRoutes {
p.registerResourceFailureNotSupportedForExpressionRoutes(udproute)
continue
}

if err := p.ingressRulesFromUDPRoute(&result, udproute); err != nil {
err = fmt.Errorf("UDPRoute %s/%s can't be routed: %w", udproute.Namespace, udproute.Name, err)
errs = append(errs, err)
Expand Down

0 comments on commit 6ce0245

Please sign in to comment.