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

feat(reader) handle _transform field #520

Merged
merged 7 commits into from
Dec 8, 2021
Merged

feat(reader) handle _transform field #520

merged 7 commits into from
Dec 8, 2021

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Nov 5, 2021

Handle the Kong state _transform field, which indicates whether data should pass through applicable transforms when imported into Kong. As deck passes data through the admin API, it will always pass through transforms. deck rejects the state if _transform=false, as it cannot satisfy this request.

AFAIK, we need take no additional action for true, and that is the default per https://docs.konghq.com/gateway-oss/2.6.x/db-less-and-declarative-config/#the-declarative-configuration-format. Rejecting false outright instead of warning since that should indicate that the file contains transformed/encrypted content, and running it through transforms again will not result in the intended config.

Fix #510

@rainest rainest requested a review from a team as a code owner November 5, 2021 18:21
@rainest rainest force-pushed the fix/transform branch 3 times, most recently from 64cc681 to 29a1c7e Compare November 5, 2021 18:35
@rainest
Copy link
Contributor Author

rainest commented Nov 5, 2021

Testing the positive case for Get() fails due to not having a Kong instance, but

11:32:38-0700 esenin $ dlv debug -- validate -s foo.yaml 
Type 'help' for list of commands.
(dlv) c
Process 3923781 has exited with status 0
(dlv) exit
11:33:10-0700 esenin $ dlv debug -- diff -s foo.yaml        
Type 'help' for list of commands.
(dlv) c
creating certificate 13c562a1-191c-4464-9b18-e5222b46035b
creating consumer yolo
creating consumer harry
creating upstream upstream1
creating target 198.51.100.11:80 for upstream 42156d4b-b339-457c-9868-60f7636b7e27
creating target 198.51.100.12:80 for upstream 42156d4b-b339-457c-9868-60f7636b7e27
creating service svc1
creating key-auth n90ts for consumer 6e6590a5-d25d-4f1c-b265-9ee6097e5bd4
creating sni demo3.example.com
creating target 198.51.100.13:80 for upstream 42156d4b-b339-457c-9868-60f7636b7e27
creating sni demo2.example.com
creating service svc2
creating sni demo1.example.com
creating service svc3
Warning: import/export of basic-authcredentials using decK doesn't work due to hashing of passwords in Kong.
creating basic-auth user1 for consumer 6e6590a5-d25d-4f1c-b265-9ee6097e5bd4
creating hmac-auth hmac-user for consumer 6e6590a5-d25d-4f1c-b265-9ee6097e5bd4
creating jwt-auth MKWeR0nu9OAUR9HrjpUG82Hbfz7ZXsIw for consumer 6e6590a5-d25d-4f1c-b265-9ee6097e5bd4
creating acl-group foo-group for consumer 6e6590a5-d25d-4f1c-b265-9ee6097e5bd4
creating route r3
creating route r2
creating route r1
creating plugin prometheus (global)
creating plugin rate-limiting for consumer 3488030a-387d-4865-b409-4216131a8a07
Summary:
  Created: 23
  Updated: 0
  Deleted: 0
Process 3924231 has exited with status 0
(dlv) exit
11:33:39-0700 esenin $ diff -u kong.yaml foo.yaml 
--- kong.yaml	2021-11-05 11:17:59.690516063 -0700
+++ foo.yaml	2021-11-05 11:32:38.433901529 -0700
@@ -1,3 +1,4 @@
+_transform: true
 _info:
   select_tags:
   - managed-by-deck

file/reader.go Outdated Show resolved Hide resolved
file/reader_test.go Outdated Show resolved Hide resolved
file/reader_test.go Outdated Show resolved Hide resolved
file/reader.go Outdated Show resolved Hide resolved
file/reader_test.go Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2021

Codecov Report

Merging #520 (4dc48ff) into main (b359c83) will increase coverage by 0.20%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #520      +/-   ##
==========================================
+ Coverage   50.47%   50.67%   +0.20%     
==========================================
  Files          72       72              
  Lines        7870     7876       +6     
==========================================
+ Hits         3972     3991      +19     
+ Misses       3549     3535      -14     
- Partials      349      350       +1     
Impacted Files Coverage Δ
file/types.go 68.47% <ø> (ø)
file/reader.go 57.35% <85.71%> (+25.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b359c83...4dc48ff. Read the comment docs.

Travis Raines and others added 7 commits November 24, 2021 13:59
Handle the Kong state _transform field, which indicates whether data
should pass through applicable transforms when imported into Kong. As
deck passes data through the admin API, it will always pass through
transforms. deck rejects the state if _transform=false, as it cannot
satisfy this request.
Co-authored-by: Michał Flendrich <michal@flendrich.pro>
@hbagdi
Copy link
Member

hbagdi commented Nov 29, 2021

@kikito Can you verify if the understanding of "_transform" here is indeed correct or not?
LGTM. Please get a +1 from someone in the gateway team on the assumption behind the PR.

@hbagdi
Copy link
Member

hbagdi commented Dec 7, 2021

@rainest can you squash all your commits into a single commit? We are good to merge after that.

@rainest rainest enabled auto-merge (squash) December 7, 2021 22:41
@rainest
Copy link
Contributor Author

rainest commented Dec 7, 2021

@hbagdi auto-merge is now set to squash mode and I've removed irrelevant commit headers from the body. It'll merge once you approve.

Copy link
Contributor

@mflendrich mflendrich left a comment

Choose a reason for hiding this comment

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

LGTM but disabled auto-merge so as to prevent the discussion on comments from being missed.

@rainest rainest merged commit 65b1bee into main Dec 8, 2021
@rainest rainest deleted the fix/transform branch December 8, 2021 16:39
AntoineJac pushed a commit that referenced this pull request Jan 23, 2024
Handle the Kong state _transform field, which indicates whether data
should pass through applicable transforms when imported into Kong. As
deck passes data through the admin API, it will always pass through
transforms. deck rejects the state if _transform=false, as it cannot
satisfy this request.

Co-authored-by: Michał Flendrich <michal@flendrich.pro>
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.

Error: (root): Additional property _transform is not allowed on decK sync
4 participants