feat(#3097): propagate translation failures for netv1.Ingress (#3138)

Introduces propagating of translation failures related to Ingress resources:
- missing service,
- missing service port.

CHANGELOG.md

@@ -122,18 +122,21 @@ Adding a new version? You'll need three changes:
   [#3121](https://github.com/Kong/kubernetes-ingress-controller/pull/3121)
 - Routes support annotations for path handling.
   [#3121](https://github.com/Kong/kubernetes-ingress-controller/pull/3121)
-- Warning events are recorded when CA secrets cannot be properly translated into Kong configuration.  
+- Warning events are recorded when CA secrets cannot be properly translated into Kong configuration.
   [#3125](https://github.com/Kong/kubernetes-ingress-controller/pull/3125)
-- Warning events are recorded when annotations in services backing a single route do not match. 
+- Warning events are recorded when annotations in services backing a single route do not match.
   [#3130](https://github.com/Kong/kubernetes-ingress-controller/pull/3130)
-- Warning events are recorded when a service's referred client-cert does not exist. 
+- Warning events are recorded when a service's referred client-cert does not exist.
   [#3137](https://github.com/Kong/kubernetes-ingress-controller/pull/3137)
 - CRDs' validations improvements: `UDPIngressRule.Port`, `IngressRule.Port` and `IngressBackend.ServiceName`
   instead of being validated in the Parser, are validated by the Kubernetes API now.
   [#3136](https://github.com/Kong/kubernetes-ingress-controller/pull/3136)
 - Gateway API: Implement port matching for routes as defined in
   [GEP-957](https://gateway-api.sigs.k8s.io/geps/gep-957/)
   [#3129](https://github.com/Kong/kubernetes-ingress-controller/pull/3129)
+- Warning events are recorded when one of `netv1.Ingress` related issues occurs
+  (e.g. backing Kubernetes service couldn't be found, matching Kubernetes service port couldn't be found).
+  [#3138](https://github.com/Kong/kubernetes-ingress-controller/pull/3138)
 
 ### Fixed
 

internal/dataplane/parser/failures.go

@@ -30,6 +30,22 @@ func NewTranslationFailure(reason string, causingObjects ...client.Object) (Tran
 		return TranslationFailure{}, fmt.Errorf("no causing objects specified, reason: %s", reason)
 	}
 
+	for _, obj := range causingObjects {
+		if obj == nil {
+			return TranslationFailure{}, errors.New("one of causing objects is nil")
+		}
+		gvk := obj.GetObjectKind().GroupVersionKind()
+		if gvk.Empty() {
+			return TranslationFailure{}, errors.New("one of causing objects has an empty GVK")
+		}
+		if obj.GetName() == "" {
+			return TranslationFailure{}, fmt.Errorf("one of causing objects (%s) has no name", gvk.String())
+		}
+		if obj.GetNamespace() == "" {
+			return TranslationFailure{}, fmt.Errorf("one of causing objects (%s) has no namespace", gvk.String())
+		}
+	}
+
 	return TranslationFailure{
 		causingObjects: causingObjects,
 		reason:         reason,
@@ -63,7 +79,7 @@ func NewTranslationFailuresCollector(logger logrus.FieldLogger) (*TranslationFai
 func (c *TranslationFailuresCollector) PushTranslationFailure(reason string, causingObjects ...client.Object) {
 	translationErr, err := NewTranslationFailure(reason, causingObjects...)
 	if err != nil {
-		c.logger.Warningf("failed to create translation failure: %w", err)
+		c.logger.WithField("translation_failure_reason", reason).Warningf("failed to create translation failure: %s", err)
 		return
 	}
 

internal/dataplane/parser/failures_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser"
@@ -15,10 +16,6 @@ import (
 
 const someValidTranslationFailureReason = "some valid reason"
 
-func someValidTranslationFailureCausingObjects() []client.Object {
-	return []client.Object{&kongv1.KongIngress{}, &kongv1.KongPlugin{}}
-}
-
 func TestTranslationFailure(t *testing.T) {
 	t.Run("is created and returns reason and causing objects", func(t *testing.T) {
 		transErr, err := parser.NewTranslationFailure(someValidTranslationFailureReason, someValidTranslationFailureCausingObjects()...)
@@ -35,12 +32,33 @@ func TestTranslationFailure(t *testing.T) {
 	})
 
 	t.Run("requires at least one causing object", func(t *testing.T) {
-		_, err := parser.NewTranslationFailure(someValidTranslationFailureReason, someValidTranslationFailureCausingObjects()[0])
+		_, err := parser.NewTranslationFailure(someValidTranslationFailureReason, validCausingObject())
 		require.NoError(t, err)
 
 		_, err = parser.NewTranslationFailure(someValidTranslationFailureReason)
 		require.Error(t, err)
 	})
+
+	t.Run("requires valid objects", func(t *testing.T) {
+		_, err := parser.NewTranslationFailure(someValidTranslationFailureReason, nil)
+		assert.Error(t, err, "expected a nil object to be rejected")
+
+		emptyGVK := validCausingObject()
+		emptyGVK.APIVersion = ""
+		emptyGVK.Kind = ""
+		_, err = parser.NewTranslationFailure(someValidTranslationFailureReason, emptyGVK)
+		assert.Error(t, err, "expected an empty GVK object to be rejected")
+
+		noName := validCausingObject()
+		noName.Name = ""
+		_, err = parser.NewTranslationFailure(someValidTranslationFailureReason, noName)
+		assert.Error(t, err, "expected an empty name object to be rejected")
+
+		noNamespace := validCausingObject()
+		noNamespace.Namespace = ""
+		_, err = parser.NewTranslationFailure(someValidTranslationFailureReason, noNamespace)
+		assert.Error(t, err, "expected an empty namespace object to be rejected")
+	})
 }
 
 func TestTranslationFailuresCollector(t *testing.T) {
@@ -93,3 +111,20 @@ func assertErrorLogs(t *testing.T, logHook *test.Hook) {
 		assert.Equalf(t, logrus.ErrorLevel, logHook.AllEntries()[i].Level, "%d-nth log entry expected to have ErrorLevel", i)
 	}
 }
+
+func someValidTranslationFailureCausingObjects() []client.Object {
+	return []client.Object{validCausingObject(), validCausingObject()}
+}
+
+func validCausingObject() *kongv1.KongPlugin {
+	return &kongv1.KongPlugin{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "KongPlugin",
+			APIVersion: kongv1.SchemeGroupVersion.String(),
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "plugin-name",
+			Namespace: "default",
+		},
+	}
+}

internal/dataplane/parser/ingressrules_test.go

@@ -383,17 +383,21 @@ func TestDoK8sServicesMatchAnnotations(t *testing.T) {
 			name: "validation fails if one service does not have all expected annotations",
 			services: []*corev1.Service{
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-service1",
+						Name:      "test-service1",
+						Namespace: corev1.NamespaceDefault,
 						Annotations: map[string]string{
 							"konghq.com/bar": "foo",
 							"konghq.com/baz": "foo",
 						},
 					},
 				},
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-service2",
+						Name:      "test-service2",
+						Namespace: corev1.NamespaceDefault,
 						Annotations: map[string]string{
 							"konghq.com/baz": "foo",
 							"konghq.com/foo": "bar",
@@ -402,8 +406,10 @@ func TestDoK8sServicesMatchAnnotations(t *testing.T) {
 					},
 				},
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-service3",
+						Name:      "test-service3",
+						Namespace: corev1.NamespaceDefault,
 						Annotations: map[string]string{
 							"konghq.com/bar": "foo",
 							"konghq.com/foo": "bar",
@@ -426,24 +432,30 @@ func TestDoK8sServicesMatchAnnotations(t *testing.T) {
 			name: "validation fails if all services have the same annotations, but not the same value",
 			services: []*corev1.Service{
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-service1",
+						Name:      "test-service1",
+						Namespace: corev1.NamespaceDefault,
 						Annotations: map[string]string{
 							"konghq.com/foo": "bar",
 						},
 					},
 				},
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-service2",
+						Name:      "test-service2",
+						Namespace: corev1.NamespaceDefault,
 						Annotations: map[string]string{
 							"konghq.com/foo": "baz",
 						},
 					},
 				},
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-service3",
+						Name:      "test-service3",
+						Namespace: corev1.NamespaceDefault,
 						Annotations: map[string]string{
 							"konghq.com/foo": "buzz",
 						},
@@ -484,6 +496,7 @@ func TestPopulateServices(t *testing.T) {
 			name: "one service to skip, one service to keep",
 			k8sServices: []*corev1.Service{
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "k8s-service-to-skip1",
 						Namespace: "test-namespace",
@@ -493,12 +506,14 @@ func TestPopulateServices(t *testing.T) {
 					},
 				},
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "k8s-service-to-skip2",
 						Namespace: "test-namespace",
 					},
 				},
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "k8s-service-to-keep1",
 						Namespace: "test-namespace",
@@ -508,6 +523,7 @@ func TestPopulateServices(t *testing.T) {
 					},
 				},
 				{
+					TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "k8s-service-to-keep2",
 						Namespace: "test-namespace",

internal/dataplane/parser/parser.go

@@ -108,7 +108,7 @@ func (p *Parser) Build() (*kongstate.KongState, []TranslationFailure) {
 	}
 
 	// generate Upstreams and Targets from service defs
-	result.Upstreams = getUpstreams(p.logger, p.storer, ingressRules.ServiceNameToServices)
+	result.Upstreams = p.getUpstreams(ingressRules.ServiceNameToServices)
 
 	// merge KongIngress with Routes, Services and Upstream
 	result.FillOverrides(p.logger, p.storer)
@@ -266,11 +266,7 @@ func findPort(svc *corev1.Service, wantPort kongstate.PortDef) (*corev1.ServiceP
 	return nil, fmt.Errorf("no suitable port found")
 }
 
-func getUpstreams(
-	log logrus.FieldLogger,
-	s store.Storer,
-	serviceMap map[string]kongstate.Service,
-) []kongstate.Upstream {
+func (p *Parser) getUpstreams(serviceMap map[string]kongstate.Service) []kongstate.Upstream {
 	upstreamDedup := make(map[string]struct{}, len(serviceMap))
 	var empty struct{}
 	upstreams := make([]kongstate.Upstream, 0, len(serviceMap))
@@ -287,22 +283,28 @@ func getUpstreams(
 				// gather the Kubernetes service for the backend
 				k8sService, ok := service.K8sServices[backend.Name]
 				if !ok {
-					log.WithField("service_name", *service.Name).Errorf("can't add target for backend %s: no kubernetes service found", backend.Name)
+					p.registerTranslationFailure(
+						fmt.Sprintf("can't add target for backend %s: no kubernetes service found", backend.Name),
+						service.Parent,
+					)
 					continue
 				}
 
 				// determine the port for the backend
 				port, err := findPort(k8sService, backend.PortDef)
 				if err != nil {
-					log.WithField("service_name", *service.Name).Errorf("can't find port for backend kubernetes service %s/%s: %v", k8sService.Namespace, k8sService.Name, err)
+					p.registerTranslationFailure(
+						fmt.Sprintf("can't find port for backend kubernetes service: %v", err),
+						k8sService, service.Parent,
+					)
 					continue
 				}
 
 				// get the new targets for this backend service
-				newTargets := getServiceEndpoints(log, s, k8sService, port)
+				newTargets := getServiceEndpoints(p.logger, p.storer, k8sService, port)
 
 				if len(newTargets) == 0 {
-					log.WithField("service_name", *service.Name).Infof("no targets could be found for kubernetes service %s/%s", k8sService.Namespace, k8sService.Name)
+					p.logger.WithField("service_name", *service.Name).Infof("no targets could be found for kubernetes service %s/%s", k8sService.Namespace, k8sService.Name)
 				}
 
 				// if weights were set for the backend then that weight needs to be
@@ -337,7 +339,7 @@ func getUpstreams(
 
 			// warn if an upstream was created with 0 targets
 			if len(targets) == 0 {
-				log.WithField("service_name", *service.Name).Infof("no targets found to create upstream")
+				p.logger.WithField("service_name", *service.Name).Infof("no targets found to create upstream")
 			}
 
 			// define the upstream including all the newly populated targets

internal/dataplane/parser/parser_test.go

@@ -837,6 +837,7 @@ func TestCACertificate(t *testing.T) {
 	t.Run("valid CACertificte is processed", func(t *testing.T) {
 		secrets := []*corev1.Secret{
 			{
+				TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "foo",
 					Namespace: "default",
@@ -872,6 +873,7 @@ func TestCACertificate(t *testing.T) {
 	t.Run("multiple CACertifictes are processed", func(t *testing.T) {
 		secrets := []*corev1.Secret{
 			{
+				TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "foo",
 					Namespace: "default",
@@ -888,6 +890,7 @@ func TestCACertificate(t *testing.T) {
 				},
 			},
 			{
+				TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "bar",
 					Namespace: "non-default",
@@ -919,6 +922,7 @@ func TestCACertificate(t *testing.T) {
 	t.Run("invalid CACertifictes are ignored", func(t *testing.T) {
 		secrets := []*corev1.Secret{
 			{
+				TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "valid-cert",
 					Namespace: "default",
@@ -935,6 +939,7 @@ func TestCACertificate(t *testing.T) {
 				},
 			},
 			{
+				TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "missing-cert-key",
 					Namespace: "non-default",
@@ -951,6 +956,7 @@ func TestCACertificate(t *testing.T) {
 				},
 			},
 			{
+				TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "missing-id-key",
 					Namespace: "non-default",
@@ -967,6 +973,7 @@ func TestCACertificate(t *testing.T) {
 				},
 			},
 			{
+				TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "expired-cert",
 					Namespace: "non-default",
@@ -1126,6 +1133,7 @@ func TestServiceClientCertificate(t *testing.T) {
 
 		services := []*corev1.Service{
 			{
+				TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "foo-svc",
 					Namespace: "default",
@@ -2840,6 +2848,7 @@ func TestDefaultBackend(t *testing.T) {
 
 		services := []*corev1.Service{
 			{
+				TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: corev1.SchemeGroupVersion.String()},
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "foo-svc",
 					Namespace: "default",