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.

Sign up for GitHub

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

Jump to bottom

internal/broker: Warn when switching to offline mode #386

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

internal/broker: Warn when switching to offline mode #386

wants to merge 1 commit into from

Conversation

MggMuggins
Copy link

If a user gives an invalid issuer URL in broker.conf, go-oidc will return an error with the HTTP response code:

Could not connect to the provider: 404 Not Found: . Starting session in offline mode.

We shouldn't leave users guessing as to the online/offline state of the broker, especially not the reason for that state choice.

I would have used Noticef here instead of Warningf but authd 0.4.1 doesn't have Noticef.

@MggMuggins MggMuggins requested a review from a team as a code owner February 11, 2025 18:53
@adombeck
Copy link
Contributor

We shouldn't leave users guessing as to the online/offline state of the broker, especially not the reason for that state choice.

Makes sense to me!

I would have used Noticef here instead of Warningf but authd 0.4.1 doesn't have Noticef.

True, but main has (since f8343ba) :) I pushed a commit which uses Noticef instead.

@MggMuggins
internal/broker: Warn when switching to offline mode
fdc2909 
If a user gives an invalid issuer URL in broker.conf, go-oidc will return
an error with the HTTP response code. This should be logged by default so
that users aren't left guessing as to the online/offline state of the broker.

Signed-off-by: Wesley Hershberger <[email protected]>
Reviewed-by: Adrian Dombeck <[email protected]>
@MggMuggins MggMuggins force-pushed the mggmuggins/warn-on-provider-connect-failure branch from 3485b6e to fdc2909 Compare February 11, 2025 21:36
@MggMuggins
Copy link
Author

Thanks, missed that 0.4.2 was in use in the gomod. Rebased this and added you in the Reviewed-by

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adombeck adombeck adombeck approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MggMuggins @adombeck