Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correct bug when consumer-group-consumer doesn't have an username #1113

Merged
merged 1 commit into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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