fix: skip consumers when using --skip-consumers with the sync command#559
Merged
fix: skip consumers when using --skip-consumers with the sync command#559
Conversation
rajkong
reviewed
Jan 6, 2022
| if err != nil { | ||
| return err | ||
| } | ||
| if dumpConfig.SkipConsumers { |
Contributor
There was a problem hiding this comment.
I am wondering if we can figure a way to get rid of these global mutable vars?
Contributor
There was a problem hiding this comment.
Not a big deal and certainly not in the scope of this PR.
hbagdi
approved these changes
Jan 14, 2022
| if err != nil { | ||
| return err | ||
| } | ||
| if dumpConfig.SkipConsumers { |
Contributor
There was a problem hiding this comment.
Not a big deal and certainly not in the scope of this PR.
Contributor
|
@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. |
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In order to have
--skip-consumerswork as expected with thesynccommand, 2 things need to happen:consumersto be ignored on fetched current stateconsumersto be ignored on generated target stateThe first bullet is currently taken care of here, which is called by the
syncMainfunction here.The second bullet is what we are currently missing.
Before the change in this PR:
Not only
deckis creating the consumer despite the flag, the fact that we are successfully ignoring thecurrentstate BUT NOT thetargetstate makes a second run fail, becausedeckfails to detect theyoloconsumer already exists and so it tries to create it:After the change in this PR:
Same goes if some consumer already exist but that's not present in the target file:
Fixes #532