Skip to content

fix: skip consumers when using --skip-consumers with the sync command#559

Merged
hbagdi merged 1 commit intomainfrom
ggabriele/sync_consumers
Jan 14, 2022
Merged

fix: skip consumers when using --skip-consumers with the sync command#559
hbagdi merged 1 commit intomainfrom
ggabriele/sync_consumers

Conversation

@GGabriele
Copy link
Collaborator

@GGabriele GGabriele commented Jan 6, 2022

In order to have --skip-consumers work as expected with the sync command, 2 things need to happen:

  • consumers to be ignored on fetched current state
  • consumers to be ignored on generated target state

The first bullet is currently taken care of here, which is called by the syncMain function here.
The second bullet is what we are currently missing.

Before the change in this PR:

$ cat kong.yaml
services:
- name: svc1
  host: mockbin.org
  retries: 5
  tags:
  - team-svc1
consumers:
- username: yolo

$ http :8001/consumers
{
    "data": [],
    "next": null
}

$ ./deck sync --skip-consumers
creating consumer yolo
Summary:
  Created: 1
  Updated: 0
  Deleted: 0

Not only deck is creating the consumer despite the flag, the fact that we are successfully ignoring the current state BUT NOT the target state makes a second run fail, because deck fails to detect the yolo consumer already exists and so it tries to create it:

$ ./deck sync --skip-consumers
creating consumer yolo
Summary:
  Created: 0
  Updated: 0
  Deleted: 0
Error: 1 errors occurred:
	while processing event: {Create} consumer yolo failed: HTTP status 409 (message: "UNIQUE violation detected on '{username=\"yolo\"}'")

After the change in this PR:

$ cat kong.yaml
services:
- name: svc1
  host: mockbin.org
  retries: 5
  tags:
  - team-svc1
consumers:
- username: yolo

$ http :8001/consumers
{
    "data": [],
    "next": null
}

$ ./deck sync --skip-consumers
Summary:
  Created: 0
  Updated: 0
  Deleted: 0

Same goes if some consumer already exist but that's not present in the target file:

$ cat kong.yaml
services:
- name: svc1
  host: mockbin.org
  tags:
  - team-svc1
  retries: 5

$ http :8001/consumers
{
    "data": [
        {
            "created_at": 1641491690,
            "custom_id": null,
            "id": "0f994481-a438-4401-a918-835d0a887050",
            "tags": [
                "managed-by-deck",
                "org-unit-42"
            ],
            "username": "yolo"
        }
    ],
    "next": null
}

$ ./deck sync --skip-consumers
Summary:
  Created: 0
  Updated: 0
  Deleted: 0

Fixes #532

@GGabriele GGabriele requested a review from a team as a code owner January 6, 2022 17:58
@GGabriele GGabriele requested review from hbagdi and rajkong January 6, 2022 18:01
if err != nil {
return err
}
if dumpConfig.SkipConsumers {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if we can figure a way to get rid of these global mutable vars?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal and certainly not in the scope of this PR.

@hbagdi hbagdi requested a review from rainest January 7, 2022 19:05
Copy link
Contributor

@hbagdi hbagdi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

if err != nil {
return err
}
if dumpConfig.SkipConsumers {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal and certainly not in the scope of this PR.

@hbagdi
Copy link
Contributor

hbagdi commented Jan 14, 2022

@GGabriele Please make sure to follow git commit conventions (https://github.com/Kong/kong/blob/master/CONTRIBUTING.md#submitting-a-patch). I'm going to go ahead and amend the commit message for this patch.

@hbagdi hbagdi merged commit b0184e8 into main Jan 14, 2022
@hbagdi hbagdi deleted the ggabriele/sync_consumers branch January 14, 2022 19:30
AntoineJac pushed a commit that referenced this pull request Jan 23, 2024
In order to have `--skip-consumers` work as expected with the `sync` command, 2 things need to happen:
- `consumers` to be ignored on fetched current state
- `consumers` to be ignored on generated target state

The first bullet is currently taken care of [here](https://github.com/Kong/deck/blob/main/dump/dump.go#L259-L260), which is called by the `syncMain` function [here](https://github.com/Kong/deck/blob/main/cmd/common.go#L189).
The second bullet is what we are currently missing.

**Before the change in this PR:**

```
$ cat kong.yaml
services:
- name: svc1
  host: mockbin.org
  retries: 5
  tags:
  - team-svc1
consumers:
- username: yolo

$ http :8001/consumers
{
    "data": [],
    "next": null
}

$ ./deck sync --skip-consumers
creating consumer yolo
Summary:
  Created: 1
  Updated: 0
  Deleted: 0

```

Not only `deck` is creating the consumer despite the flag, the fact that we are successfully ignoring the `current` state BUT NOT the `target` state makes a second run fail, because `deck` fails to detect the `yolo` consumer already exists and so it tries to create it:

```
$ ./deck sync --skip-consumers
creating consumer yolo
Summary:
  Created: 0
  Updated: 0
  Deleted: 0
Error: 1 errors occurred:
	while processing event: {Create} consumer yolo failed: HTTP status 409 (message: "UNIQUE violation detected on '{username=\"yolo\"}'")
```

**After the change in this PR:**

```
$ cat kong.yaml
services:
- name: svc1
  host: mockbin.org
  retries: 5
  tags:
  - team-svc1
consumers:
- username: yolo

$ http :8001/consumers
{
    "data": [],
    "next": null
}

$ ./deck sync --skip-consumers
Summary:
  Created: 0
  Updated: 0
  Deleted: 0
```

Same goes if some consumer already exist but that's not present in the target file:
```
$ cat kong.yaml
services:
- name: svc1
  host: mockbin.org
  tags:
  - team-svc1
  retries: 5

$ http :8001/consumers
{
    "data": [
        {
            "created_at": 1641491690,
            "custom_id": null,
            "id": "0f994481-a438-4401-a918-835d0a887050",
            "tags": [
                "managed-by-deck",
                "org-unit-42"
            ],
            "username": "yolo"
        }
    ],
    "next": null
}

$ ./deck sync --skip-consumers
Summary:
  Created: 0
  Updated: 0
  Deleted: 0
```

Fixes #532
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deck sync creates consumers even with --skip-consumers

3 participants