Skip to content

Commit

Permalink
fix: correct bug when consumer-group-consumer doesn't have an username
Browse files Browse the repository at this point in the history
  • Loading branch information
GGabriele committed Nov 21, 2023
1 parent cc24866 commit f7e0a17
Show file tree
Hide file tree
Showing 9 changed files with 222 additions and 10 deletions.
11 changes: 10 additions & 1 deletion state/consumer_group_consumers.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ func (k *ConsumerGroupConsumersCollection) Add(consumer ConsumerGroupConsumer) e
if !utils.Empty(consumer.Consumer.Username) {
searchBy = append(searchBy, *consumer.Consumer.Username)
}
if !utils.Empty(consumer.Consumer.CustomID) {
searchBy = append(searchBy, *consumer.Consumer.CustomID)
}
_, err := getConsumerGroupConsumer(txn, *consumer.ConsumerGroup.ID, searchBy...)
if err == nil {
return fmt.Errorf("inserting consumerGroupConsumer %v: %w", consumer.Console(), ErrAlreadyExists)
Expand Down Expand Up @@ -132,7 +135,13 @@ func getConsumerGroupConsumer(txn *memdb.Txn, consumerGroupID string, IDs ...str

for _, id := range IDs {
for _, consumer := range consumers {
if id == *consumer.Consumer.ID || id == *consumer.Consumer.Username {
var username string
if consumer.Consumer.Username != nil {
username = *consumer.Consumer.Username
} else {
username = *consumer.Consumer.CustomID
}
if id == *consumer.Consumer.ID || id == username {
return &ConsumerGroupConsumer{ConsumerGroupConsumer: *consumer.DeepCopy()}, nil
}
}
Expand Down
5 changes: 4 additions & 1 deletion state/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,10 @@ func (c1 *ConsumerGroupConsumer) Identifier() string {
// Console returns an entity's identity in a human
// readable string.
func (c1 *ConsumerGroupConsumer) Console() string {
return *c1.ConsumerGroupConsumer.Consumer.Username
if c1.ConsumerGroupConsumer.Consumer.Username != nil {
return *c1.ConsumerGroupConsumer.Consumer.Username
}
return *c1.ConsumerGroupConsumer.Consumer.CustomID
}

// Equal returns true if c1 and c2 are equal.
Expand Down
33 changes: 33 additions & 0 deletions tests/integration/dump_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func Test_Dump_SelectTags_30(t *testing.T) {
Expand Down Expand Up @@ -251,3 +252,35 @@ func Test_Dump_KonnectRename(t *testing.T) {
})
}
}

func Test_Dump_ConsumerGroupConsumersWithCustomID(t *testing.T) {
runWhen(t, "enterprise", ">=3.0.0")
setup(t)

require.NoError(t, sync("testdata/sync/028-consumer-group-consumers-custom_id/kong.yaml"))

var output string
flags := []string{"-o", "-", "--with-id"}
output, err := dump(flags...)
assert.NoError(t, err)

expected, err := readFile("testdata/sync/028-consumer-group-consumers-custom_id/kong.yaml")
assert.NoError(t, err)
assert.Equal(t, expected, output)
}

func Test_Dump_ConsumerGroupConsumersWithCustomID_Konnect(t *testing.T) {
runWhen(t, "konnect", "")
setup(t)

require.NoError(t, sync("testdata/sync/028-consumer-group-consumers-custom_id/kong.yaml"))

var output string
flags := []string{"-o", "-", "--with-id"}
output, err := dump(flags...)
assert.NoError(t, err)

expected, err := readFile("testdata/dump/003-consumer-group-consumers-custom_id/konnect.yaml")
assert.NoError(t, err)
assert.Equal(t, expected, output)
}
15 changes: 15 additions & 0 deletions tests/integration/reset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/kong/deck/utils"
"github.com/kong/go-kong/kong"
"github.com/stretchr/testify/require"
)

var caCert = &kong.CACertificate{
Expand Down Expand Up @@ -106,3 +107,17 @@ func Test_Reset_SkipCACert_3x(t *testing.T) {
})
}
}

func Test_Reset_ConsumerGroupConsumersWithCustomID(t *testing.T) {
runWhenEnterpriseOrKonnect(t, ">=3.0.0")
setup(t)

client, err := getTestClient()
if err != nil {
t.Fatalf(err.Error())
}

require.NoError(t, sync("testdata/sync/028-consumer-group-consumers-custom_id/kong.yaml"))
reset(t)
testKongState(t, client, false, utils.KongRawState{}, nil)
}
73 changes: 73 additions & 0 deletions tests/integration/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4714,3 +4714,76 @@ func Test_Sync_DoNotUpdateCreatedAt(t *testing.T) {
// plugins do not have an updated_at field
// consumers do not have an updated_at field
}

// test scope:
// - 3.0.0+
// - konnect
func Test_Sync_ConsumerGroupConsumersWithCustomID(t *testing.T) {
t.Setenv("DECK_KONNECT_CONTROL_PLANE_NAME", "default")
runWhenEnterpriseOrKonnect(t, ">=3.0.0")
setup(t)

client, err := getTestClient()
if err != nil {
t.Fatalf(err.Error())
}

expectedState := utils.KongRawState{
ConsumerGroups: []*kong.ConsumerGroupObject{
{
ConsumerGroup: &kong.ConsumerGroup{
ID: kong.String("48df7cd3-1cd0-4e53-af73-8f57f257be18"),
Name: kong.String("cg1"),
},
Consumers: []*kong.Consumer{
{
ID: kong.String("bcb296c3-22bb-46f6-99c8-4828af750b77"),
CustomID: kong.String("foo"),
},
},
},
{
ConsumerGroup: &kong.ConsumerGroup{
ID: kong.String("1a81dc83-5329-4666-8ae7-8a966e62d076"),
Name: kong.String("cg2"),
},
Consumers: []*kong.Consumer{
{
ID: kong.String("562bf5c7-a7d9-4338-84dd-2c1064fb7f67"),
Username: kong.String("foo"),
},
},
},
{
ConsumerGroup: &kong.ConsumerGroup{
ID: kong.String("d140f9cc-227e-4872-8b0b-639f6922dfb0"),
Name: kong.String("cg3"),
},
Consumers: []*kong.Consumer{
{
ID: kong.String("7906968b-cd89-4a87-8dda-94678e7106b2"),
Username: kong.String("bar"),
CustomID: kong.String("custom_bar"),
},
},
},
},
Consumers: []*kong.Consumer{
{
ID: kong.String("bcb296c3-22bb-46f6-99c8-4828af750b77"),
CustomID: kong.String("foo"),
},
{
ID: kong.String("562bf5c7-a7d9-4338-84dd-2c1064fb7f67"),
Username: kong.String("foo"),
},
{
ID: kong.String("7906968b-cd89-4a87-8dda-94678e7106b2"),
Username: kong.String("bar"),
CustomID: kong.String("custom_bar"),
},
},
}
require.NoError(t, sync("testdata/sync/028-consumer-group-consumers-custom_id/kong.yaml"))
testKongState(t, client, false, expectedState, nil)
}
11 changes: 11 additions & 0 deletions tests/integration/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,17 @@ func runWhenKongOrKonnect(t *testing.T, kongSemverRange string) {
kong.RunWhenKong(t, kongSemverRange)
}

func runWhenEnterpriseOrKonnect(t *testing.T, kongSemverRange string) {
t.Helper()

if os.Getenv("DECK_KONNECT_EMAIL") != "" &&
os.Getenv("DECK_KONNECT_PASSWORD") != "" &&
os.Getenv("DECK_KONNECT_TOKEN") != "" {
return
}
kong.RunWhenEnterprise(t, kongSemverRange, kong.RequiredFeatures{})
}

func runWhen(t *testing.T, mode string, semverRange string) {
t.Helper()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
_format_version: "3.0"
_konnect:
control_plane_name: default
consumer_groups:
- id: 48df7cd3-1cd0-4e53-af73-8f57f257be18
name: cg1
- id: 1a81dc83-5329-4666-8ae7-8a966e62d076
name: cg2
- id: d140f9cc-227e-4872-8b0b-639f6922dfb0
name: cg3
consumers:
- custom_id: custom_bar
groups:
- id: d140f9cc-227e-4872-8b0b-639f6922dfb0
name: cg3
id: 7906968b-cd89-4a87-8dda-94678e7106b2
username: bar
- custom_id: foo
groups:
- id: 48df7cd3-1cd0-4e53-af73-8f57f257be18
name: cg1
id: bcb296c3-22bb-46f6-99c8-4828af750b77
- groups:
- id: 1a81dc83-5329-4666-8ae7-8a966e62d076
name: cg2
id: 562bf5c7-a7d9-4338-84dd-2c1064fb7f67
username: foo
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
_format_version: "3.0"
consumer_groups:
- id: 48df7cd3-1cd0-4e53-af73-8f57f257be18
name: cg1
- id: 1a81dc83-5329-4666-8ae7-8a966e62d076
name: cg2
- id: d140f9cc-227e-4872-8b0b-639f6922dfb0
name: cg3
consumers:
- custom_id: custom_bar
groups:
- id: d140f9cc-227e-4872-8b0b-639f6922dfb0
name: cg3
id: 7906968b-cd89-4a87-8dda-94678e7106b2
username: bar
- custom_id: foo
groups:
- id: 48df7cd3-1cd0-4e53-af73-8f57f257be18
name: cg1
id: bcb296c3-22bb-46f6-99c8-4828af750b77
- groups:
- id: 1a81dc83-5329-4666-8ae7-8a966e62d076
name: cg2
id: 562bf5c7-a7d9-4338-84dd-2c1064fb7f67
username: foo
32 changes: 24 additions & 8 deletions types/consumer_group_consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (s *consumerGroupConsumerCRUD) Create(ctx context.Context, arg ...crud.Arg)
ctx, s.client, consumer.ConsumerGroup.ID, consumer.Consumer.ID,
)
} else {
_, err = s.client.ConsumerGroupConsumers.Create(ctx, consumer.ConsumerGroup.ID, consumer.Consumer.Username)
_, err = s.client.ConsumerGroupConsumers.Create(ctx, consumer.ConsumerGroup.ID, consumer.Consumer.ID)
}
if err != nil {
return nil, err
Expand All @@ -65,7 +65,7 @@ func (s *consumerGroupConsumerCRUD) Delete(ctx context.Context, arg ...crud.Arg)
if s.isKonnect {
err = konnect.DeleteConsumerGroupMember(ctx, s.client, consumer.ConsumerGroup.ID, consumer.Consumer.ID)
} else {
err = s.client.ConsumerGroupConsumers.Delete(ctx, consumer.ConsumerGroup.ID, consumer.Consumer.Username)
err = s.client.ConsumerGroupConsumers.Delete(ctx, consumer.ConsumerGroup.ID, consumer.Consumer.ID)
}
if err != nil {
return nil, err
Expand All @@ -90,7 +90,7 @@ func (s *consumerGroupConsumerCRUD) Update(ctx context.Context, arg ...crud.Arg)
)
} else {
err = s.client.ConsumerGroupConsumers.Delete(
ctx, consumer.ConsumerGroup.ID, consumer.Consumer.Username,
ctx, consumer.ConsumerGroup.ID, consumer.Consumer.ID,
)
}
if err != nil {
Expand All @@ -104,7 +104,7 @@ func (s *consumerGroupConsumerCRUD) Update(ctx context.Context, arg ...crud.Arg)
)
} else {
_, err = s.client.ConsumerGroupConsumers.Create(
ctx, consumer.ConsumerGroup.ID, consumer.Consumer.Username,
ctx, consumer.ConsumerGroup.ID, consumer.Consumer.ID,
)
}
if err != nil {
Expand Down Expand Up @@ -150,8 +150,16 @@ func (d *consumerGroupConsumerDiffer) Deletes(handler func(crud.Event) error) er
func (d *consumerGroupConsumerDiffer) deleteConsumerGroupConsumer(
consumer *state.ConsumerGroupConsumer,
) (*crud.Event, error) {
var nameOrID string
if consumer.Consumer.ID != nil {
nameOrID = *consumer.Consumer.ID
} else if consumer.Consumer.Username != nil {
nameOrID = *consumer.Consumer.Username
} else {
nameOrID = *consumer.Consumer.CustomID
}
_, err := d.targetState.ConsumerGroupConsumers.Get(
*consumer.Consumer.Username, *consumer.ConsumerGroup.ID,
nameOrID, *consumer.ConsumerGroup.ID,
)
if errors.Is(err, state.ErrNotFound) {
return &crud.Event{
Expand All @@ -162,7 +170,7 @@ func (d *consumerGroupConsumerDiffer) deleteConsumerGroupConsumer(
}
if err != nil {
return nil, fmt.Errorf("looking up consumerGroupConsumer %q: %w",
*consumer.Consumer.Username, err)
nameOrID, err)
}
return nil, nil
}
Expand Down Expand Up @@ -192,8 +200,16 @@ func (d *consumerGroupConsumerDiffer) createUpdateConsumerGroupConsumer(
consumer *state.ConsumerGroupConsumer,
) (*crud.Event, error) {
consumerCopy := &state.ConsumerGroupConsumer{ConsumerGroupConsumer: *consumer.DeepCopy()}
var nameOrID string
if consumer.Consumer.ID != nil {
nameOrID = *consumer.Consumer.ID
} else if consumer.Consumer.Username != nil {
nameOrID = *consumer.Consumer.Username
} else {
nameOrID = *consumer.Consumer.CustomID
}
currentConsumer, err := d.currentState.ConsumerGroupConsumers.Get(
*consumer.Consumer.Username, *consumer.ConsumerGroup.ID,
nameOrID, *consumer.ConsumerGroup.ID,
)
if errors.Is(err, state.ErrNotFound) {
return &crud.Event{
Expand All @@ -204,7 +220,7 @@ func (d *consumerGroupConsumerDiffer) createUpdateConsumerGroupConsumer(
}
if err != nil {
return nil, fmt.Errorf("error looking up consumerGroupConsumer %v: %w",
*currentConsumer.Consumer.Username, err)
nameOrID, err)
}

// found, check if update needed
Expand Down

0 comments on commit f7e0a17

Please sign in to comment.