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

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

Merged
merged 3 commits into from
May 18, 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
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