fix: routes with conflicting services

CHANGELOG.md

@@ -2,12 +2,13 @@
 
 <!---
 Adding a new version? You'll need three changes:
-* Add the ToC link, like "[1.2.3](#123]".
+* Add the ToC link, like "[1.2.3](#123)".
 * Add the section header, like "## [1.2.3]".
 * Add the diff link, like "[2.7.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v1.2.2...v1.2.3".
   This is all the way at the bottom. It's the thing we always forget.
 --->
 
+ - [2.8.0](#280)
  - [2.7.0](#270)
  - [2.6.0](#260)
  - [2.5.0](#250)
@@ -60,6 +61,17 @@ Adding a new version? You'll need three changes:
  - [0.0.5](#005)
  - [0.0.4 and prior](#004-and-prior)
 
+## [2.8.0]
+
+> Release date: TBD
+
+### Fixed
+
+- The controller now logs an error for and skips multi-Service rules that have
+  inconsistent Service annotations. Previously this issue prevented the
+  controller from applying configuration until corrected.
+  [#2988](https://github.com/Kong/kubernetes-ingress-controller/pull/2988)
+
 ## [2.7.0]
 
 > Release date: 2022-09-26
@@ -1991,6 +2003,7 @@ Please read the changelog and test in your environment.
  - The initial versions  were rapildy iterated to deliver
    a working ingress controller.
 
+[2.8.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.7.0...v2.8.0
 [2.7.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.6.0...v2.7.0
 [2.6.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.5.0...v2.6.0
 [2.5.0]: https://github.com/kong/kubernetes-ingress-controller/compare/v2.4.2...v2.5.0

internal/dataplane/parser/ingressrules.go

@@ -1,7 +1,6 @@
 package parser
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/kong/go-kong/kong"
@@ -41,7 +40,11 @@ func mergeIngressRules(objs ...ingressRules) ingressRules {
 	return result
 }
 
-func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer) error {
+// populateServices populates the ServiceNameToServices map with additional information
+// and returns a map of services to be skipped.
+func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer) map[string]interface{} {
+	serviceNamesToSkip := make(map[string]interface{})
+
 	// populate Kubernetes Service
 	for key, service := range ir.ServiceNameToServices {
 		if service.K8sServices == nil {
@@ -52,12 +55,13 @@ func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer)
 		// and all the annotations in use across all services (when applicable).
 		k8sServices, seenAnnotations := getK8sServicesForBackends(log, s, service.Namespace, service.Backends)
 
-		// if the Kubernetes services have been deemed invalid, no need to continue
-		// they will all be dropped until the problem has been rectified.
+		// if the Kubernetes services have been deemed invalid, log an error message
+		// and skip the current service.
 		if !servicesAllUseTheSameKongAnnotations(log, k8sServices, seenAnnotations) {
-			return fmt.Errorf("the Kubernetes Services %v cannot have different sets of konghq.com annotations. "+
-				"These Services are used in the same Gateway Route BackendRef together to create the Kong Service %s"+
-				"and must use the same Kong annotations", k8sServices, *service.Name)
+			// The Kong services not having all the k8s services correctly annotated must be marked
+			// as to be skipped.
+			serviceNamesToSkip[key] = nil
+			continue
 		}
 
 		for _, k8sService := range k8sServices {
@@ -91,8 +95,7 @@ func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer)
 		// now be cached.
 		ir.ServiceNameToServices[key] = service
 	}
-
-	return nil
+	return serviceNamesToSkip
 }
 
 type SecretNameToSNIs map[string][]string

internal/dataplane/parser/ingressrules_test.go

@@ -4,12 +4,14 @@ import (
 	"bytes"
 	"testing"
 
+	"github.com/kong/go-kong/kong"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	netv1beta1 "k8s.io/api/networking/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
@@ -467,3 +469,103 @@ func TestDoK8sServicesMatchAnnotations(t *testing.T) {
 		})
 	}
 }
+
+func TestPopulateServices(t *testing.T) {
+	testCases := []struct {
+		name                   string
+		k8sServices            []*corev1.Service
+		serviceNamesToServices map[string]kongstate.Service
+		serviceNamesToSkip     map[string]interface{}
+	}{
+		{
+			name: "one service to skip, one service to keep",
+			k8sServices: []*corev1.Service{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "k8s-service-to-skip1",
+						Namespace: "test-namespace",
+						Annotations: map[string]string{
+							"konghq.com/foo": "bar",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "k8s-service-to-skip2",
+						Namespace: "test-namespace",
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "k8s-service-to-keep1",
+						Namespace: "test-namespace",
+						Annotations: map[string]string{
+							"konghq.com/foo": "bar",
+						},
+					},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "k8s-service-to-keep2",
+						Namespace: "test-namespace",
+						Annotations: map[string]string{
+							"konghq.com/foo": "bar",
+						},
+					},
+				},
+			},
+			serviceNamesToServices: map[string]kongstate.Service{
+				"service-to-skip": {
+					Service: kong.Service{
+						Name: pointer.StringPtr("service-to-skip"),
+					},
+					Namespace: "test-namespace",
+					Backends: []kongstate.ServiceBackend{
+						{
+							Name:      "k8s-service-to-skip1",
+							Namespace: "test-namespace",
+						},
+						{
+							Name:      "k8s-service-to-skip2",
+							Namespace: "test-namespace",
+						},
+					},
+				},
+				"service-to-keep": {
+					Service: kong.Service{
+						Name: pointer.StringPtr("service-to-skip"),
+					},
+					Namespace: "test-namespace",
+					Backends: []kongstate.ServiceBackend{
+						{
+							Name:      "k8s-service-to-keep1",
+							Namespace: "test-namespace",
+						},
+						{
+							Name:      "k8s-service-to-keep2",
+							Namespace: "test-namespace",
+						},
+					},
+				},
+			},
+			serviceNamesToSkip: map[string]interface{}{
+				"service-to-skip": nil,
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		t.Run(tc.name, func(t *testing.T) {
+			ingressRules := newIngressRules()
+			fakeStore, err := store.NewFakeStore(store.FakeObjects{
+				Services: tc.k8sServices,
+			})
+			require.NoError(t, err)
+			ingressRules.ServiceNameToServices = tc.serviceNamesToServices
+			servicesToBeSkipped := ingressRules.populateServices(logrus.New(), fakeStore)
+			require.Equal(t, tc.serviceNamesToSkip, servicesToBeSkipped)
+		})
+	}
+}

internal/dataplane/parser/parser.go

@@ -80,15 +80,18 @@ func (p *Parser) Build() (*kongstate.KongState, error) {
 		p.ingressRulesFromTLSRoutes(),
 	)
 
-	// populate any Kubernetes Service objects relevant objects
-	if err := ingressRules.populateServices(p.logger, p.storer); err != nil {
-		return nil, err
-	}
+	// populate any Kubernetes Service objects relevant objects and get the
+	// services to be skipped because of annotations inconsistency
+	servicesToBeSkipped := ingressRules.populateServices(p.logger, p.storer)
 
 	// add the routes and services to the state
 	var result kongstate.KongState
-	for _, service := range ingressRules.ServiceNameToServices {
-		result.Services = append(result.Services, service)
+	for key, service := range ingressRules.ServiceNameToServices {
+		// if the service doesn't need to be skipped, then add it to the
+		// list of services.
+		if _, ok := servicesToBeSkipped[key]; !ok {
+			result.Services = append(result.Services, service)
+		}
 	}
 
 	// generate Upstreams and Targets from service defs