From da9117908fd091659b22f041b9ab058cb62ddbf6 Mon Sep 17 00:00:00 2001 From: Prashansa Kulshrestha Date: Thu, 5 Sep 2024 18:00:00 +0530 Subject: [PATCH] fix: added fix to allow one consumer to be in >1 consumer-groups. (#140) * fix: added fix to allow one consumer to be in >1 consumer-groups. gdr version 1.39.2 introduced a bug where a consumer was limited to only one consumer-group. Adding the consumer in >1 group was resulting in an error on deck. This PR adds the fix for the same and an integration test. See: https://github.com/Kong/deck/issues/1384 * fix: scoped the test to >3.4.0 * chore: clarified test scope in integration sync_test * tests: added test for gateway v2.8.x for >1 consumer-group for a consumer --- pkg/state/consumer_group_consumers.go | 15 +++- tests/integration/sync_test.go | 86 +++++++++++++++++++ tests/integration/test_utils.go | 1 + .../kong.yaml | 9 ++ .../kong3x.yaml | 9 ++ 5 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 tests/integration/testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong.yaml create mode 100644 tests/integration/testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong3x.yaml diff --git a/pkg/state/consumer_group_consumers.go b/pkg/state/consumer_group_consumers.go index 12b6108..317565a 100644 --- a/pkg/state/consumer_group_consumers.go +++ b/pkg/state/consumer_group_consumers.go @@ -151,12 +151,21 @@ func getConsumerGroupConsumer(txn *memdb.Txn, consumerGroupID string, IDs ...str for _, id := range IDs { for _, index := range indexes { - res, err := txn.First(consumerGroupConsumerTableName, index, id) + res, err := txn.Get(consumerGroupConsumerTableName, index, id) if err != nil { return nil, err } - if res != nil { - consumer := res.(*ConsumerGroupConsumer) + + for { + resultValue := res.Next() + if resultValue == nil { + break + } + consumer, ok := resultValue.(*ConsumerGroupConsumer) + if !ok { + break + } + if *consumer.ConsumerGroup.ID == consumerGroupID { return consumer, nil } diff --git a/tests/integration/sync_test.go b/tests/integration/sync_test.go index e2a65dd..ad00e60 100644 --- a/tests/integration/sync_test.go +++ b/tests/integration/sync_test.go @@ -5667,3 +5667,89 @@ func Test_Sync_PluginAutoFields(t *testing.T) { "Should error out due to missing provision_key") }) } + +// test scope: +// - enterprise +// - >=3.4.0 +func Test_Sync_MoreThanOneConsumerGroupForOneConsumer(t *testing.T) { + runWhen(t, "enterprise", ">=3.4.0") + setup(t) + + client, err := getTestClient() + require.NoError(t, err) + + expectedState := utils.KongRawState{ + ConsumerGroups: []*kong.ConsumerGroupObject{ + { + ConsumerGroup: &kong.ConsumerGroup{ + Name: kong.String("group1"), + }, + Consumers: []*kong.Consumer{ + { + Username: kong.String("my-test-consumer"), + }, + }, + }, + { + ConsumerGroup: &kong.ConsumerGroup{ + Name: kong.String("group2"), + }, + Consumers: []*kong.Consumer{ + { + Username: kong.String("my-test-consumer"), + }, + }, + }, + }, + Consumers: []*kong.Consumer{ + { + Username: kong.String("my-test-consumer"), + }, + }, + } + require.NoError(t, sync("testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong3x.yaml")) + testKongState(t, client, false, expectedState, nil) +} + +// test scope: +// - enterprise +// - 2.8.0 +func Test_Sync_MoreThanOneConsumerGroupForOneConsumer_2_8(t *testing.T) { + runWhen(t, "enterprise", ">=2.8.0 <3.0.0") + setup(t) + + client, err := getTestClient() + require.NoError(t, err) + + expectedState := utils.KongRawState{ + ConsumerGroups: []*kong.ConsumerGroupObject{ + { + ConsumerGroup: &kong.ConsumerGroup{ + Name: kong.String("group1"), + }, + Consumers: []*kong.Consumer{ + { + Username: kong.String("my-test-consumer"), + }, + }, + }, + { + ConsumerGroup: &kong.ConsumerGroup{ + Name: kong.String("group2"), + }, + Consumers: []*kong.Consumer{ + { + Username: kong.String("my-test-consumer"), + }, + }, + }, + }, + Consumers: []*kong.Consumer{ + { + Username: kong.String("my-test-consumer"), + }, + }, + } + require.NoError(t, sync("testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong.yaml")) + testKongState(t, client, false, expectedState, nil) +} diff --git a/tests/integration/test_utils.go b/tests/integration/test_utils.go index 8cddbad..b128789 100644 --- a/tests/integration/test_utils.go +++ b/tests/integration/test_utils.go @@ -228,6 +228,7 @@ func testKongState(t *testing.T, client *kong.Client, isKonnect bool, cmpopts.IgnoreFields(kong.Vault{}, "ID", "CreatedAt", "UpdatedAt"), cmpopts.IgnoreFields(kong.Certificate{}, "ID", "CreatedAt"), cmpopts.IgnoreFields(kong.SNI{}, "ID", "CreatedAt"), + cmpopts.IgnoreFields(kong.Consumer{}, "CreatedAt", "ID"), cmpopts.IgnoreFields(kong.ConsumerGroup{}, "CreatedAt", "ID"), cmpopts.IgnoreFields(kong.ConsumerGroupPlugin{}, "CreatedAt", "ID"), cmpopts.IgnoreFields(kong.KeyAuth{}, "ID", "CreatedAt"), diff --git a/tests/integration/testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong.yaml b/tests/integration/testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong.yaml new file mode 100644 index 0000000..641698b --- /dev/null +++ b/tests/integration/testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong.yaml @@ -0,0 +1,9 @@ +_format_version: "1.0" +consumer_groups: +- name: group1 +- name: group2 +consumers: +- username: my-test-consumer + groups: + - name: group1 + - name: group2 diff --git a/tests/integration/testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong3x.yaml b/tests/integration/testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong3x.yaml new file mode 100644 index 0000000..e5be41f --- /dev/null +++ b/tests/integration/testdata/sync/xxx-more-than-one-consumer-group-with-a-consumer/kong3x.yaml @@ -0,0 +1,9 @@ +_format_version: "3.0" +consumer_groups: +- name: group1 +- name: group2 +consumers: +- username: my-test-consumer + groups: + - name: group1 + - name: group2