-
Notifications
You must be signed in to change notification settings - Fork 21
fix(flex-linux-setup): setup imports aui_webhook.ldif #2440
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
Merged
duttarnab
merged 1 commit into
main
from
flex-libux-setup-missing-web-hook-contents-2439
Nov 11, 2025
+1
−1
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
🧩 Analysis chain
Webhook LDIF import is conditional on first-time setup only.
The webhook LDIF file is only imported when
client_check_result['2001.'] == -1(line 491), meaning it will only be imported during fresh installations when the Admin UI Web Client doesn't exist.This could be problematic because:
Consider one of these approaches:
Verify whether this import should happen only on first-time setup or also during upgrades:
🏁 Script executed:
Length of output: 8601
🏁 Script executed:
Length of output: 6885
Webhook LDIF import is only on first install and skipped entirely during updates.
The webhook LDIF is only imported when
client_check_result['2001.'] == -1(line 491), meaning it only runs during fresh installation when the Admin UI Web Client doesn't exist.Critical issues:
Upgrade scenarios: The
--update-admin-uipath (line 820) callsdownload_files(),install_config_api_plugin(), andunpack_gluu_admin_ui_archive()but never callsinstall_gluu_admin_ui(), which contains the webhook import. Webhook entries are completely skipped on updates.Re-running setup: If the client exists, the entire block (lines 491-519) is skipped, so webhook entries won't be imported if missing from an existing setup.
Inconsistent with Docker bootstrap: The docker-admin-ui bootstrap imports webhook.ldif unconditionally as part of
ldif_files, showing the intended behavior.Fix: Either move webhook LDIF import outside the conditional block, or ensure it's also imported in the update path by calling
install_gluu_admin_ui()or a separate webhook import method.🤖 Prompt for AI Agents
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.
@coderabbitai Webhook LDIF should only be imported for fresh installations. Docker update is different than linux update
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.
@devrimyatar Thank you for the clarification! I understand now that the webhook LDIF import on fresh installations only is the intended design for Linux setup, and that Docker and Linux update processes have different requirements. The current implementation correctly reflects this behavior.
I appreciate you explaining the distinction between Docker and Linux update workflows—this helps ensure future reviews align with the project's design decisions.
✏️ Learnings added