fix: fix panic when handling KongConsumer without username

examples/kong-consumer-key-auth.yaml

@@ -13,8 +13,8 @@ kind: KongConsumer
 metadata:
   name: consumer1
   annotations:
-      kubernetes.io/ingress.class: kong
-# username: consumer1
+    kubernetes.io/ingress.class: kong
+username: consumer1
 credentials:
 - consumer1-auth
 ---

internal/dataplane/deckgen/consumer_cmp.go

@@ -0,0 +1,50 @@
+package deckgen
+
+import (
+	"strings"
+
+	"github.com/kong/go-database-reconciler/pkg/file"
+)
+
+type fConsumerByUsernameAndCustomID []file.FConsumer
+
+func (f fConsumerByUsernameAndCustomID) Len() int      { return len(f) }
+func (f fConsumerByUsernameAndCustomID) Swap(i, j int) { f[i], f[j] = f[j], f[i] }
+func (f fConsumerByUsernameAndCustomID) Less(i, j int) bool {
+	if f[i].Username == nil && f[j].Username != nil {
+		return true
+	}
+	if f[i].Username != nil && f[j].Username == nil {
+		return false
+	}
+	if f[i].Username != nil && f[j].Username != nil {
+		switch cmp := strings.Compare(*f[i].Username, *f[j].Username); cmp {
+		case -1:
+			return true
+		case 1:
+			return false
+		case 0:
+			break
+		}
+	}
+
+	// Both usernames are equal, compare custom_id.
+	if f[i].CustomID == nil && f[j].CustomID != nil {
+		return true
+	}
+	if f[i].CustomID != nil && f[j].CustomID == nil {
+		return false
+	}
+	if f[i].CustomID != nil && f[j].CustomID != nil {
+		switch cmp := strings.Compare(*f[i].CustomID, *f[j].CustomID); cmp {
+		case -1:
+			return true
+		case 1:
+			return false
+		case 0:
+			break
+		}
+	}
+
+	return false
+}

internal/dataplane/deckgen/consumer_cmp_test.go

@@ -0,0 +1,111 @@
+package deckgen
+
+import (
+	"sort"
+	"testing"
+
+	"github.com/kong/go-database-reconciler/pkg/file"
+	"github.com/kong/go-kong/kong"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestConsumerCmp(t *testing.T) {
+	testCases := []struct {
+		name     string
+		input    []file.FConsumer
+		expected []file.FConsumer
+	}{
+		{
+			name: "sort by username",
+			input: []file.FConsumer{
+				{
+					Consumer: kong.Consumer{
+						Username: kong.String("b"),
+					},
+				},
+				{
+					Consumer: kong.Consumer{
+						Username: kong.String("a"),
+					},
+				},
+			},
+			expected: []file.FConsumer{
+				{
+					Consumer: kong.Consumer{
+						Username: kong.String("a"),
+					},
+				},
+				{
+					Consumer: kong.Consumer{
+						Username: kong.String("b"),
+					},
+				},
+			},
+		},
+		{
+			name: "sort by custom_id",
+			input: []file.FConsumer{
+				{
+					Consumer: kong.Consumer{
+						CustomID: kong.String("b"),
+					},
+				},
+				{
+					Consumer: kong.Consumer{
+						CustomID: kong.String("a"),
+					},
+				},
+			},
+			expected: []file.FConsumer{
+				{
+					Consumer: kong.Consumer{
+						CustomID: kong.String("a"),
+					},
+				},
+				{
+					Consumer: kong.Consumer{
+						CustomID: kong.String("b"),
+					},
+				},
+			},
+		},
+		{
+			name: "sort by username and custom_id",
+			input: []file.FConsumer{
+				{
+					Consumer: kong.Consumer{
+						Username: kong.String("b"),
+						CustomID: kong.String("b"),
+					},
+				},
+				{
+					Consumer: kong.Consumer{
+						Username: kong.String("a"),
+						CustomID: kong.String("a"),
+					},
+				},
+			},
+			expected: []file.FConsumer{
+				{
+					Consumer: kong.Consumer{
+						Username: kong.String("a"),
+						CustomID: kong.String("a"),
+					},
+				},
+				{
+					Consumer: kong.Consumer{
+						Username: kong.String("b"),
+						CustomID: kong.String("b"),
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			sort.Sort(fConsumerByUsernameAndCustomID(tc.input))
+			assert.Equal(t, tc.expected, tc.input)
+		})
+	}
+}

internal/dataplane/deckgen/generate.go

@@ -152,11 +152,12 @@ func ToDeckContent(
 	for _, c := range k8sState.Consumers {
 		consumer := file.FConsumer{Consumer: c.Consumer}
 
-		// if a consumer with no username is provided deck wont be able to process it, but we shouldn't
-		// fail the rest of the deckgen either or this will result in one bad consumer being capable of
-		// stopping all updates to the Kong Admin API.
-		if consumer.Username == nil {
-			logger.Error(nil, "Invalid consumer received (username was empty)")
+		// If a consumer with no username and no custom_id is provided deck wont be able to process it,
+		// but we shouldn't fail the rest of the deckgen either or this will result in one bad consumer
+		// being capable of stopping all updates to the Kong Admin API.
+		// This shouldn't happen as we enforce either of those field being present in CRD CEL validation rules.
+		if consumer.Username == nil && consumer.CustomID == nil {
+			logger.Error(nil, "Invalid consumer received (username and custom_id were empty)")
 			continue
 		}
 
@@ -191,9 +192,7 @@ func ToDeckContent(
 		}
 		content.Consumers = append(content.Consumers, consumer)
 	}
-	sort.SliceStable(content.Consumers, func(i, j int) bool {
-		return strings.Compare(*content.Consumers[i].Username, *content.Consumers[j].Username) > 0
-	})
+	sort.Stable(fConsumerByUsernameAndCustomID(content.Consumers))
 
 	// convert vaults.
 	for _, v := range k8sState.Vaults {

internal/dataplane/kongstate/kongstate.go

@@ -422,9 +422,24 @@ func (ks *KongState) getPluginRelations(cacheStore store.Storer, log logr.Logger
 	}
 
 	for _, c := range ks.Consumers {
+		if c.Username == nil && c.CustomID == nil {
+			// Either Custom ID or username has to be filled so this shouldn't
+			// happen but let's log here just in case.
+			log.Error(nil, "no username or custom_id specified", "consumer", c.K8sKongConsumer.Name)
+			break
+		}
+
+		var identifier string
+		switch {
+		case c.Username != nil:
+			identifier = *c.Username
+		case c.CustomID != nil:
+			identifier = *c.CustomID
+		}
+
 		pluginList := annotations.ExtractNamespacedKongPluginsFromAnnotations(c.K8sKongConsumer.GetAnnotations())
 		for _, plugin := range pluginList {
-			addRelation(&c.K8sKongConsumer, plugin, *c.Username, ConsumerRelation)
+			addRelation(&c.K8sKongConsumer, plugin, identifier, ConsumerRelation)
 		}
 	}
 

internal/dataplane/kongstate/kongstate_test.go

@@ -533,10 +533,62 @@ func TestGetPluginRelations(t *testing.T) {
 				"ns2:baz": {Route: []string{"bar-route"}, ConsumerGroup: []string{"bar-consumer-group"}},
 			},
 		},
+		{
+			name: "consumer with custom_id and a plugin attached",
+			args: args{
+				state: KongState{
+					Consumers: []Consumer{
+						{
+							Consumer: kong.Consumer{
+								CustomID: kong.String("1234-1234"),
+							},
+							K8sKongConsumer: kongv1.KongConsumer{
+								ObjectMeta: metav1.ObjectMeta{
+									Namespace: "default",
+									Annotations: map[string]string{
+										annotations.AnnotationPrefix + annotations.PluginsKey: "rate-limiting-1",
+									},
+								},
+							},
+						},
+					},
+					Plugins: []Plugin{
+						{
+							Plugin: kong.Plugin{
+								Name: kong.String("rate-limiting"),
+							},
+							K8sParent: &kongv1.KongPlugin{
+								ObjectMeta: metav1.ObjectMeta{
+									Namespace: "default",
+									Name:      "rate-limiting-1",
+								},
+								PluginName: "rate-limiting",
+							},
+						},
+						{
+							Plugin: kong.Plugin{
+								Name: kong.String("basic-auth"),
+							},
+							K8sParent: &kongv1.KongPlugin{
+								ObjectMeta: metav1.ObjectMeta{
+									Namespace: "default",
+									Name:      "basic-auth-1",
+								},
+								PluginName: "basic-auth",
+							},
+						},
+					},
+				},
+			},
+			want: map[string]util.ForeignRelations{
+				"default:rate-limiting-1": {Consumer: []string{"1234-1234"}},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			store, _ := store.NewFakeStore(store.FakeObjects{})
+			store, err := store.NewFakeStore(store.FakeObjects{})
+			require.NoError(t, err)
 			if got := tt.args.state.getPluginRelations(store, logr.Discard()); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("getPluginRelations() = %v, want %v", got, tt.want)
 			}

internal/dataplane/testdata/golden/consumer-group-example-ee/default_golden.yaml

@@ -1,11 +1,11 @@
 _format_version: "3.0"
 consumer_group_consumers:
+- consumer: consumer-1
+  consumer_group: consumer-group-1
 - consumer: consumer-2
   consumer_group: consumer-group-1
 - consumer: consumer-2
   consumer_group: consumer-group-2
-- consumer: consumer-1
-  consumer_group: consumer-group-1
 consumer_groups:
 - id: 876a1c78-f75a-570e-a918-26506344add0
   name: consumer-group-2
@@ -22,17 +22,17 @@ consumer_groups:
   - k8s-group:configuration.konghq.com
   - k8s-version:v1beta1
 consumers:
-- id: 71168e5f-1d0b-5465-8bb9-a3b032fbc4c4
+- id: e23d9ef8-1cc4-5b55-abd1-db89aa90346c
   tags:
-  - k8s-name:consumer-2
+  - k8s-name:consumer-1
   - k8s-kind:KongConsumer
   - k8s-group:configuration.konghq.com
   - k8s-version:v1
-  username: consumer-2
-- id: e23d9ef8-1cc4-5b55-abd1-db89aa90346c
+  username: consumer-1
+- id: 71168e5f-1d0b-5465-8bb9-a3b032fbc4c4
   tags:
-  - k8s-name:consumer-1
+  - k8s-name:consumer-2
   - k8s-kind:KongConsumer
   - k8s-group:configuration.konghq.com
   - k8s-version:v1
-  username: consumer-1
+  username: consumer-2

internal/dataplane/testdata/golden/kongconsumer-with-custom-id/default_golden.yaml

@@ -0,0 +1,9 @@
+_format_version: "3.0"
+consumers:
+- custom_id: 1234-1234
+  tags:
+  - k8s-name:consumer
+  - k8s-namespace:consumer-namespace
+  - k8s-kind:KongConsumer
+  - k8s-group:configuration.konghq.com
+  - k8s-version:v1

internal/dataplane/testdata/golden/kongconsumer-with-custom-id/expression-routes-on_golden.yaml

@@ -0,0 +1,9 @@
+_format_version: "3.0"
+consumers:
+- custom_id: 1234-1234
+  tags:
+  - k8s-name:consumer
+  - k8s-namespace:consumer-namespace
+  - k8s-kind:KongConsumer
+  - k8s-group:configuration.konghq.com
+  - k8s-version:v1

internal/dataplane/testdata/golden/kongconsumer-with-custom-id/expression-routes-on_settings.yaml

@@ -0,0 +1,2 @@
+feature_flags:
+  ExpressionRoutes: true

internal/dataplane/testdata/golden/kongconsumer-with-custom-id/in.yaml

@@ -0,0 +1,9 @@
+---
+apiVersion: configuration.konghq.com/v1
+kind: KongConsumer
+metadata:
+  name: consumer
+  namespace: consumer-namespace
+  annotations:
+    kubernetes.io/ingress.class: kong
+custom_id: "1234-1234"