-
Notifications
You must be signed in to change notification settings - Fork 3
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(diff+file): avoid filling defaults in config for kong #133
Conversation
de5b40e
to
82f8e41
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #133 +/- ##
==========================================
+ Coverage 42.76% 42.78% +0.01%
==========================================
Files 75 75
Lines 8820 8809 -11
==========================================
- Hits 3772 3769 -3
+ Misses 4586 4579 -7
+ Partials 462 461 -1 ☔ View full report in Codecov by Sentry. |
510522f
to
1d85e9c
Compare
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.
Does this change make the filling defaults only run for diff
printing differences but not to sync
really updating configuration? Would this approach still require changes in Kong/go-kong?
Correct @randmonkey , I added examples of how the change affects deck in the PR description.
No, this PR replaces the other one I opened, I will close the old PR if/once this is approved |
Does |
not that I'm aware of, but that's a good question for @Kong/team-apiops maybe |
the previous logic was filling defaults in the configuration that was passed to Kong. This was problematic, especially where nils were populated as defaults, e.g. if a shorthand_field was passed with some value and the corresponding new field is auto-populated as `nil` by decK , the auto-populated nil value would take precedence in Kong thus causing the shorthand_field to be ignored (https://konghq.atlassian.net/browse/KAG-5157). This change applies the default values only to configurations used for diff, the original configuration is always passed to Kong as is.
* add tests for plugin filling default values --------- Co-authored-by: samugi <[email protected]>
9a81082
to
decf4d3
Compare
failing CI because httpbin.org is down ... I'll try to rerun / merge later |
The version of go-database-reconciler fixes the default config filling of plugins that causes config mismatch in case deprecated and new fields are present together. Refer: Kong/go-database-reconciler#133
Summary
the previous logic was filling defaults in the configuration that was passed to Kong. This was problematic, especially where
nils
were populated as defaults, e.g. if ashorthand_field
was passed with some value and the corresponding new field was auto-populated asnil
by decK , the auto-populated nil value would take precedence in Kong thus causing the shorthand_field to be ignored (see linked issues).This change applies the default values only to configurations used for
diff
, the original configuration from the file is passed to Kong as is.Configuration
Before the change
After the change
Issues resolved
KAG-5157
KAG-5210
Documentation
Testing