Skip to content

Commit

Permalink
fix: correct consumers handling when custom_id is used
Browse files Browse the repository at this point in the history
Right now, when a consumer has the `username` equal to the
`custom_id` of another consumer, decK fails because of the
way the consumers are handled in the internal in-memory DB.

```
_format_version: "3.0"
consumers:
- custom_id: foo
  id: ce49186d-7670-445d-a218-897631b29ada
  username: Foo
- custom_id: bar
  id: 7820f383-7b77-4fcc-af7f-14ff3e256693
  username: foo
```

```
$ deck sync
Error: inserting consumer into state: inserting consumer Foo: entity already exists
```

This commit fixes this defect by modifying the way consumers are
looked up in the in-memory DB.
  • Loading branch information
GGabriele committed Jul 27, 2023
1 parent 24ef372 commit 2b81957
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 7 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@
- [v0.2.0](#v020)
- [v0.1.0](#v010)

## Unreleased

### Fixes

- Fix Consumers handling when a consumer's `custom_id` is equal to the `username` of another consumer.
[#986](https://github.com/Kong/deck/pull/986)

## [v1.24.0]

> Release date: 2023/07/24
Expand Down
26 changes: 19 additions & 7 deletions state/consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,23 @@ func (k *ConsumersCollection) Add(consumer Consumer) error {
if !utils.Empty(consumer.Username) {
searchBy = append(searchBy, *consumer.Username)
}

// search separately by id+username and by custom_id.
//
// This is because the custom_id is unique, but it may be equal to
// the username of another consumer. If we search by both id+username and
// custom_id, we may get a false positive.
_, err := getConsumer(txn, []string{"Username", "id"}, searchBy...)
if err == nil {
return fmt.Errorf("inserting consumer %v: %w", consumer.Console(), ErrAlreadyExists)
} else if !errors.Is(err, ErrNotFound) {
return err
}

if !utils.Empty(consumer.CustomID) {
searchBy = append(searchBy, *consumer.CustomID)
searchBy = []string{*consumer.CustomID}
}
_, err := getConsumer(txn, searchBy...)
_, err = getConsumer(txn, []string{"CustomID"}, searchBy...)
if err == nil {
return fmt.Errorf("inserting consumer %v: %w", consumer.Console(), ErrAlreadyExists)
} else if !errors.Is(err, ErrNotFound) {
Expand All @@ -72,10 +85,9 @@ func (k *ConsumersCollection) Add(consumer Consumer) error {
return nil
}

func getConsumer(txn *memdb.Txn, IDs ...string) (*Consumer, error) {
func getConsumer(txn *memdb.Txn, indexes []string, IDs ...string) (*Consumer, error) {
for _, id := range IDs {
res, err := multiIndexLookupUsingTxn(txn, consumerTableName,
[]string{"Username", "id", "CustomID"}, id)
res, err := multiIndexLookupUsingTxn(txn, consumerTableName, indexes, id)
if errors.Is(err, ErrNotFound) {
continue
}
Expand All @@ -99,7 +111,7 @@ func (k *ConsumersCollection) Get(userNameOrID string) (*Consumer, error) {

txn := k.db.Txn(false)
defer txn.Abort()
return getConsumer(txn, userNameOrID)
return getConsumer(txn, []string{"Username", "id"}, userNameOrID)
}

// Update udpates an existing consumer.
Expand Down Expand Up @@ -128,7 +140,7 @@ func (k *ConsumersCollection) Update(consumer Consumer) error {
}

func deleteConsumer(txn *memdb.Txn, userNameOrID string) error {
consumer, err := getConsumer(txn, userNameOrID)
consumer, err := getConsumer(txn, []string{"Username", "id"}, userNameOrID)
if err != nil {
return err
}
Expand Down
30 changes: 30 additions & 0 deletions tests/integration/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3519,3 +3519,33 @@ func Test_Sync_CreateCertificateWithSNIs(t *testing.T) {
},
}, ignoredFields)
}

// test scope:
// - 3.0.0+
// - konnect
func Test_Sync_ConsumersWithCustomIDAndUsername(t *testing.T) {
runWhenKongOrKonnect(t, ">=3.0.0")

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

err = sync("testdata/sync/024-consumers-with-custom_id-and-username/kong3x.yaml")
require.NoError(t, err)

testKongState(t, client, false, utils.KongRawState{
Consumers: []*kong.Consumer{
{
ID: kong.String("ce49186d-7670-445d-a218-897631b29ada"),
Username: kong.String("Foo"),
CustomID: kong.String("foo"),
},
{
ID: kong.String("7820f383-7b77-4fcc-af7f-14ff3e256693"),
Username: kong.String("foo"),
CustomID: kong.String("bar"),
},
},
}, nil)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
_format_version: "3.0"
consumers:
- custom_id: foo
id: ce49186d-7670-445d-a218-897631b29ada
username: Foo
- custom_id: bar
id: 7820f383-7b77-4fcc-af7f-14ff3e256693
username: foo

0 comments on commit 2b81957

Please sign in to comment.