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

Allow overriding of keycloak root credentials for 2024.11.1 upgrade path #2843

Merged
merged 6 commits into from
Nov 12, 2024

Conversation

viniciusdc
Copy link
Contributor

@viniciusdc viniciusdc commented Nov 11, 2024

Reference Issues or PRs

closes #2833

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

How to test this PR?

Any other comments?

@viniciusdc
Copy link
Contributor Author

needs to be validated if the keycloak_admin.get_server_info works as a first layer credential check

exit()
else:
# Handle other exceptions
print(f"[red bold]An unexpected error occurred: {e}[/red bold]")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the whole stack trace be in red bold?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see it doesn't print out the stack trace. I agree with the comments on https://stackoverflow.com/a/1483488/9848141 that repr would be better than str(e) to give more info about what is happening.

if "invalid_grant" in str(e):
print(
"[red bold]Failed to connect to the Keycloak server.[/red bold]\n"
"[yellow]This may occur if the default admin credentials have been changed for security reasons.[/yellow]\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this makes it sound like the user did something wrong or optional when in reality changing the security credentials should be the default behaviors. I'll suggest an edit.

Copy link
Member

@Adam-D-Lewis Adam-D-Lewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

viniciusdc and others added 3 commits November 12, 2024 13:35
Co-authored-by: Adam Lewis <23342526+Adam-D-Lewis@users.noreply.github.com>
@viniciusdc
Copy link
Contributor Author

This is how it looks like now:
Captura de tela de 2024-11-12 13 40 57

@viniciusdc viniciusdc merged commit e3b8ec1 into main Nov 12, 2024
25 of 26 checks passed
@viniciusdc viniciusdc deleted the allow-keycloak-root-pass-upgrade branch November 12, 2024 23:48
viniciusdc added a commit that referenced this pull request Nov 13, 2024
… path (#2843)

Co-authored-by: Adam Lewis <23342526+Adam-D-Lewis@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

[BUG] - Nebari Upgrade Keycloak Client Cannot Connect if initial_root_password changed
2 participants