Skip to content

fix(flex-linux-setup): setup imports aui_webhook.ldif#2440

Merged
duttarnab merged 1 commit intomainfrom
flex-libux-setup-missing-web-hook-contents-2439
Nov 11, 2025
Merged

fix(flex-linux-setup): setup imports aui_webhook.ldif#2440
duttarnab merged 1 commit intomainfrom
flex-libux-setup-missing-web-hook-contents-2439

Conversation

@devrimyatar
Copy link
Contributor

@devrimyatar devrimyatar commented Nov 11, 2025

Closes #2439

Summary by CodeRabbit

  • Bug Fixes
    • Improved Admin UI installation process to ensure all required configuration files are properly imported during setup.

Signed-off-by: Mustafa Baser <mbaser@mail.com>
@devrimyatar devrimyatar added bug Something isn't working comp-flex-linux-setup Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality labels Nov 11, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 11, 2025

📝 Walkthrough

Walkthrough

This pull request extends the Admin UI installation process to import an additional LDIF file containing webhook configuration. The install_gluu_admin_ui function now includes admin_ui_web_hook_ldif_fn in the LDIF import step, ensuring webhook-related directory entries are properly created during setup.

Changes

Cohort / File(s) Change Summary
Admin UI LDIF Import Enhancement
flex-linux-setup/flex_linux_setup/flex_setup.py
Extended install_gluu_admin_ui to import webhook LDIF file (admin_ui_web_hook_ldif_fn) alongside existing client configuration in the LDIF import operation

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Verify that admin_ui_web_hook_ldif_fn is properly defined and accessible before this point in the code
  • Confirm the LDIF file path and content are correct for the webhook configuration
  • Ensure the import operation order doesn't introduce any dependency issues

Possibly related PRs

Suggested reviewers

  • duttarnab

Poem

🐰 A webhook whispers through the LDIF flow,
Now Admin UI features all shall glow,
One more file imported with care,
Missing contents now everywhere! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding aui_webhook.ldif import to the flex-linux-setup, which aligns with the PR objective.
Linked Issues check ✅ Passed The PR successfully addresses issue #2439 by importing aui_webhook.ldif during Admin UI setup, matching the stated objective.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the aui_webhook.ldif import in the Admin UI installation process, with no out-of-scope modifications detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch flex-libux-setup-missing-web-hook-contents-2439

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7cd9d77 and e295cc0.

📒 Files selected for processing (1)
  • flex-linux-setup/flex_linux_setup/flex_setup.py (1 hunks)
🔇 Additional comments (1)
flex-linux-setup/flex_linux_setup/flex_setup.py (1)

337-337: File verified and implementation is correct.

The aui_webhook.ldif file exists in flex-linux-setup/flex_linux_setup/templates/. Both the variable definition at line 337 and its usage at line 519 are correct. The webhook LDIF is imported conditionally when the Admin UI client is first created (client_check_result['2001.'] == -1), which is the appropriate behavior for initial setup.

print("\033[1mAdmin UI Web Client Secret:\033[0m", Config.admin_ui_client_pw)

self.dbUtils.import_ldif([client_tmp_fn])
self.dbUtils.import_ldif([client_tmp_fn, self.admin_ui_web_hook_ldif_fn])
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 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:

  1. Upgrade scenarios: If a user upgrades an existing Flex installation, the client already exists, so this block is skipped and the webhook entries won't be imported.
  2. Partial state: If the client exists but webhook entries are missing (the exact issue fix(jans-linux-setup): missing contnets in table auiFeatures #2439 describes), re-running setup won't fix it.

Consider one of these approaches:

  • Import the webhook LDIF unconditionally (outside the conditional block)
  • Add a separate check to verify webhook entries exist before skipping the import
  • Import in both branches (when client exists and when it doesn't)

Verify whether this import should happen only on first-time setup or also during upgrades:


🏁 Script executed:

#!/bin/bash
# Description: Check if there are other installation/upgrade paths that handle webhook import

# Search for other references to webhook LDIF
rg -n "webhook" --type py -C 3

# Search for upgrade-related logic
rg -n "update|upgrade" --type py -C 3 flex-linux-setup/

# Check if there's logic to handle existing clients
ast-grep --pattern $'if client_check_result[$_] == 1:
  $$$'

Length of output: 8601


🏁 Script executed:

#!/bin/bash
# Get full context around the webhook import logic
sed -n '480,530p' flex-linux-setup/flex_linux_setup/flex_setup.py

echo "---"
echo "Search for all import_ldif calls:"
rg -n "import_ldif" --type py -B 2 -A 2

echo "---"
echo "Check update_admin_ui path execution:"
sed -n '820,850p' flex-linux-setup/flex_linux_setup/flex_setup.py

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:

  1. Upgrade scenarios: The --update-admin-ui path (line 820) calls download_files(), install_config_api_plugin(), and unpack_gluu_admin_ui_archive() but never calls install_gluu_admin_ui(), which contains the webhook import. Webhook entries are completely skipped on updates.

  2. 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.

  3. 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
In flex-linux-setup/flex_linux_setup/flex_setup.py around lines 491-519 (and
reference line ~820), the webhook LDIF import is guarded by a check that only
runs on fresh installs so webhook entries are skipped during --update-admin-ui
and when re-running setup; fix by making the webhook import unconditional or
ensuring the update path invokes it: either move the
self.dbUtils.import_ldif([client_tmp_fn, self.admin_ui_web_hook_ldif_fn]) call
outside the client_exists conditional so it always runs, or add a dedicated
import step invoked from the --update-admin-ui flow (e.g., call
install_gluu_admin_ui() or a new import_webhook_ldif() from the update-admin-ui
branch after unpack_gluu_admin_ui_archive()) so webhook.ldif is always applied
on updates and re-runs.

Copy link
Contributor Author

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

Copy link
Contributor

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
Learnt from: devrimyatar
Repo: GluuFederation/flex PR: 2440
File: flex-linux-setup/flex_linux_setup/flex_setup.py:519-519
Timestamp: 2025-11-11T15:17:34.639Z
Learning: In flex-linux-setup/flex_linux_setup/flex_setup.py, the webhook LDIF (aui_webhook.ldif) should only be imported during fresh Admin UI installations, not during updates. The import is correctly guarded by the check `client_check_result['2001.'] == -1` which ensures it only runs when the Admin UI Web Client doesn't already exist. Docker update workflows differ from Linux update workflows in this regard.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@duttarnab duttarnab merged commit 5c2f301 into main Nov 11, 2025
10 of 12 checks passed
@duttarnab duttarnab deleted the flex-libux-setup-missing-web-hook-contents-2439 branch November 11, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working comp-flex-linux-setup Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(jans-linux-setup): missing contnets in table auiFeatures

2 participants