-
Notifications
You must be signed in to change notification settings - Fork 40
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: fill config values and defaults in shorthand fields #458
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #458 +/- ##
==========================================
+ Coverage 59.61% 59.79% +0.17%
==========================================
Files 71 71
Lines 4408 4455 +47
==========================================
+ Hits 2628 2664 +36
- Misses 1166 1172 +6
- Partials 614 619 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Added a unit test for the recent changes @GGabriele. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments here and there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the remaining bits, LGTM.
As of now, deprecated fields are renamed to shorthand_fields in all schemas, as per the conventions. Till now, we do not fill any values for these fields while attempting to fill config records for plugins. This creates a visualisation problem in decK. Everytime, a deck diff or deck sync command is issued, it shows that the deprecated field values are changed and need to be updated, thus running an unnecessary update process each time and also confusing end users. Check #1251: Kong/deck#1251 for clarity. This fix attempts to fill defaults for the shorthand_fields, retaining their values, if passed. Also, since shorthand_fields take priority over normal/nested fields in the gateway, if any changes are detected in shorthand_fields, it is backfilled to nested fields as well.
68f01de
to
3a072ec
Compare
As of now, deprecated fields are renamed to shorthand_fields in all schemas, as per the conventions. Till now, we do not fill any values for these fields while attempting to fill config records for plugins. This creates a visualisation problem in decK. Everytime, a deck diff or deck sync command is issued, it shows that the deprecated field values are changed and need to be updated, thus running an unnecessary update process each time and also confusing end users.
Check Kong/deck#1251 for details.
This fix attempts to fill defaults for the shorthand_fields, retaining their values, if passed. Also, since shorthand_fields take priority over normal/nested fields in the gateway, if any changes are detected in shorthand_fields, it is backfilled to nested fields as well.