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: remove all selected tags when using dump #766

Merged
merged 2 commits into from
Oct 26, 2022

Conversation

GGabriele
Copy link
Collaborator

The expected behaviour for deck dump when using --select-tag is the following:

  1. selected tags information should be summarized at the top of the configuration file inside the _info structure
  2. selected tags must be removed from entity-level tags

Right now decK doesn't perform (2) consistently, stripping out selected tags from some entities, but not from some others. For example:

$ deck dump --select-tag test-tag --yes

$ cat kong.yaml
_format_version: "3.0"
_info:
  defaults: {}
  select_tags:
  - test-tag
services:
- connect_timeout: 60000
  enabled: true
  host: mockbin.org
  name: svc1
  port: 80
  protocol: http
  read_timeout: 60000
  retries: 5
  routes:
  - https_redirect_status_code: 301
    name: r1
    path_handling: v0
    paths:
    - /r1
    preserve_host: false
    protocols:
    - http
    - https
    regex_priority: 0
    request_buffering: true
    response_buffering: true
    strip_path: true
    tags:
    - another-test-tag
  tags:
  - test-tag
  write_timeout: 60000

Here, test-tag is correctly stripped out from the route entity, which now has only the another-test-tag tag, but the test-tag is still included in the service entity.

This PR makes sure that selected tags are removed from all entities, so that this information will only be included in the _info structure.

@GGabriele GGabriele requested a review from a team October 11, 2022 11:08
@GGabriele GGabriele temporarily deployed to Configure ci October 11, 2022 11:08 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 11, 2022 11:10 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 11, 2022 11:10 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 11, 2022 11:12 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 11, 2022 11:12 Inactive
@GGabriele GGabriele requested a review from hbagdi October 12, 2022 07:31
aboudreault
aboudreault previously approved these changes Oct 18, 2022
@GGabriele GGabriele temporarily deployed to Configure ci October 26, 2022 07:15 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 26, 2022 07:15 Inactive
The expected behaviour for `deck dump` when using `--select-tag`
is the following:
1. selected tags information should be summarized at the top of
   the configuration file inside the `_info` structure
2. selected tags must be removed from entity-level tags

Right now decK doesn't perform (2) consistently, stripping out
selected tags from some entities, but not from some others.
For example:

```
$ deck dump --select-tag test-tag --yes

$ cat kong.yaml
_format_version: "3.0"
_info:
  defaults: {}
  select_tags:
  - test-tag
services:
- connect_timeout: 60000
  enabled: true
  host: mockbin.org
  name: svc1
  port: 80
  protocol: http
  read_timeout: 60000
  retries: 5
  routes:
  - https_redirect_status_code: 301
    name: r1
    path_handling: v0
    paths:
    - /r1
    preserve_host: false
    protocols:
    - http
    - https
    regex_priority: 0
    request_buffering: true
    response_buffering: true
    strip_path: true
    tags:
    - another-test-tag
  tags:
  - test-tag
  write_timeout: 60000
```

Here, `test-tag` is correctly stripped out from the route
entity, which now has only the `another-test-tag` tag, but the
`test-tag` is still included in the service entity.

This PR makes sure that selected tags are removed from all
entities, so that this information will only be included in the
`_info` structure.
@GGabriele GGabriele temporarily deployed to Configure ci October 26, 2022 07:22 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 26, 2022 07:23 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 26, 2022 07:29 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 26, 2022 07:29 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 26, 2022 07:32 Inactive
@GGabriele GGabriele temporarily deployed to Configure ci October 26, 2022 07:32 Inactive
@GGabriele GGabriele merged commit a391708 into main Oct 26, 2022
@GGabriele GGabriele deleted the fix/no-tags-on-entities branch October 26, 2022 07:39
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.

2 participants