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(diag) expose unparseable config errors #5773

Merged
merged 5 commits into from
Apr 4, 2024
Merged

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Mar 28, 2024

What this PR does / why we need it:

Add a diagnostic endpoint for raw config errors to simplify problem identification when Kong does not return flattened errors.

Attempt to log the message field if we cannot unmarshal a config error into the proper type.

Which issue this PR fixes:

Fix #5762

Special notes for your reviewer:

Logs from practical testing:

2024-03-28T19:45:41Z	error	could not fully parse config error	{"message": "declarative config is invalid: {targets={[2]=\"uniqueness violation: 'targets' entity with primary key set to '172fcf76-2116-5155-ae84-e534423f5354' already declared\"}}"}
2024-03-28T19:45:41Z	error	Failed parsing resource errors	{"url": "https://10.244.0.6:8444", "update_strategy": "InMemory", "error": "could not unmarshal config error: json: cannot unmarshal object into Go struct field ConfigError.flattened_errors of type []sendconfig.FlatEntityError"}
2024-03-28T19:45:41Z	error	dataplane-synchronizer	Could not update kong admin	{"error": "performing update for https://10.244.0.7:8444 failed: failed posting new config to /config: got status code 400\nperforming update for https://10.244.0.6:8444 failed: failed posting new config to /config: got status code 400"}

Diag output looks like endpoints.txt using resources in test.yaml.txt


I wanted to log the basic message field as well, to provide a more accessible (no need to enable config dumps) record of mystery errors. The body observed in the original report does have a message field:

{                      
  "message": "declarative config is invalid: {targets={[2]=\"uniqueness violation: targets entity with primary key set to 0500bbf6-db53-52da-ad42-d13937d1e29c already declared\"}}",
  "flattened_errors": {},
  "code": 14,
  "fields": {
    "targets": [
      null,
      "uniqueness violation: targets entity with primary key set to 0500bbf6-db53-52da-ad42-d13937d1e29c already declared"
    ]
  },
  "name": "invalid declarative configuration"
}

We unmarshal into a struct that has a message field and omits the rest if missing, but it's still failing for some reason. The lazier map[string]interface{}{} unmarshal hopefully handles this if we've at least gotten a Kong JSON config error and not a generic 500 or whatever.

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 47.91667% with 25 lines in your changes are missing coverage. Please review.

Project coverage is 74.0%. Comparing base (4954d25) to head (b312ebb).
Report is 1 commits behind head on main.

Files Patch % Lines
...al/dataplane/sendconfig/inmemory_error_handling.go 0.0% 8 Missing and 1 partial ⚠️
internal/diagnostics/server.go 18.1% 9 Missing ⚠️
internal/dataplane/sendconfig/dbmode.go 20.0% 4 Missing ⚠️
internal/dataplane/sendconfig/sendconfig.go 80.0% 2 Missing ⚠️
internal/dataplane/sendconfig/inmemory.go 66.6% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #5773     +/-   ##
=======================================
+ Coverage   71.4%   74.0%   +2.6%     
=======================================
  Files        176     176             
  Lines      18185   18207     +22     
=======================================
+ Hits       12986   13480    +494     
+ Misses      4248    3733    -515     
- Partials     951     994     +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rainest rainest marked this pull request as ready for review March 28, 2024 20:53
@rainest rainest requested a review from a team as a code owner March 28, 2024 20:53
internal/util/configdiag.go Outdated Show resolved Hide resolved
internal/dataplane/sendconfig/sendconfig.go Outdated Show resolved Hide resolved
@rainest
Copy link
Contributor Author

rainest commented Mar 29, 2024

1b91b2c and the changelog were supposed to be there originally but I forgot to push the commit

@rainest rainest enabled auto-merge (squash) April 1, 2024 20:19
czeslavo
czeslavo previously approved these changes Apr 3, 2024
rainest and others added 5 commits April 3, 2024 17:06
Add a diagnostic endpoint for raw config errors to simplify problem
identification when Kong does not return flattened errors.
Co-authored-by: Patryk Małek <patryk.malek@konghq.com>
@rainest
Copy link
Contributor Author

rainest commented Apr 4, 2024

Accidentally included some local temp change I'd made to make a test run forever so I could check some things 🤦

@rainest rainest merged commit 2570997 into main Apr 4, 2024
38 checks passed
@rainest rainest deleted the feat/error-fallback branch April 4, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose un-parseable config post error content
4 participants