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

Improve audience mismatch debugging #5078

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

ddl-rliu
Copy link
Contributor

@ddl-rliu ddl-rliu commented Mar 19, 2024

Tracking issue

Improved flyte error messages #3502

Why are the changes needed?

When there is an audience mismatch during the verifyClaims step in the authzserver, the error message details the provided audience, but not the expected audience.

What changes were proposed in this pull request?

Include the expected audience(s) in the error message. This will greatly improve ease of debugging.

How was this patch tested?

Old message:

{"json":{"src":"handlers.go:303"},"level":"info","msg":"Failed to parse Access Token from context. 
Will attempt to find IDToken. Error: invalid audience [&{[account] ... 2024-03-22 17:08:22 +0000 
UTC 2024-03-22 16:53:22 +0000 UTC 0001-01-01 00:00:00 +0000 UTC ...}]","ts":"2024-03-22T16:53:52Z"}

New message:

{"json":{"src":"handlers.go:303"},"level":"info","msg":"Failed to parse Access Token from context. 
Will attempt to find IDToken. Error: invalid audience [&{[account] ... 2024-03-22 17:08:22 +0000 
UTC 2024-03-22 16:53:22 +0000 UTC 0001-01-01 00:00:00 +0000 UTC ...}], wanted ["some-audience"]",
"ts":"2024-03-22T16:53:52Z"}

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. enhancement New feature or request labels Mar 19, 2024
Signed-off-by: ddl-rliu <richard.liu@dominodatalab.com>
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Mar 20, 2024
Copy link

codecov bot commented Mar 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.81%. Comparing base (eb24107) to head (7556640).
Report is 9 commits behind head on master.

❗ Current head 7556640 differs from pull request most recent head e6abf49. Consider uploading reports for the commit e6abf49 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5078      +/-   ##
==========================================
+ Coverage   58.98%   59.81%   +0.82%     
==========================================
  Files         645      463     -182     
  Lines       55590    37643   -17947     
==========================================
- Hits        32791    22515   -10276     
+ Misses      20204    13437    -6767     
+ Partials     2595     1691     -904     
Flag Coverage Δ
unittests 59.81% <ø> (+0.82%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Mar 20, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 25, 2024
@wild-endeavor wild-endeavor merged commit b74f705 into flyteorg:master Mar 25, 2024
48 of 70 checks passed
Copy link

welcome bot commented Mar 25, 2024

Congrats on merging your first pull request! 🎉

yubofredwang pushed a commit to yubofredwang/flyte that referenced this pull request Mar 26, 2024
Signed-off-by: ddl-rliu <richard.liu@dominodatalab.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants