Repository navigation
feat: Keycloak セットアップスクリプトの改善とコマンド追加 - #52
Conversation
- Keycloak の起動待機ロジックを改善（最大60秒待機） - セットアップスクリプトでレルムとクライアントの存在確認を追加 - Makefile に keycloak-setup と keycloak-import コマンドを追加 - docker-compose.yml の環境変数を整理 変更により、Keycloak の初期設定がより確実に実行されるようになりました。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. WalkthroughThis PR centralizes Node frontend runners in the root Makefile (adds NODE_RUNNER, BUN, FRONTEND_DIR), moves frontend install/build/typecheck/test targets under that scheme, adds install_ci and clean targets, updates help text, adjusts Keycloak compose env flags, updates Keycloak setup script to change admin password and create a default user, and adds a frontend Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant Make as Makefile
participant Node as Node runner ($(NODE_RUNNER)/$(BUN))
participant Frontend as Frontend (frontend/control-plane)
rect rgb(230, 248, 230)
Dev->>Make: make before-commit
Make->>Make: run lint_text, format_check
Make->>Node: $(NODE_RUNNER) run typecheck (in $(FRONTEND_DIR))
Node-->>Make: typecheck result
alt SKIP_FRONTEND_BUILD not set
Make->>Node: $(NODE_RUNNER) run build (in $(FRONTEND_DIR))
Node-->>Make: build result
end
Make-->>Dev: aggregated status
end
note right of Node `#f8f0d6`: install_ci / install targets run\n$(BUN) install at root and in $(FRONTEND_DIR)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
コードレビュー完了PR 52 のレビューが完了しました。 主な評価✅ 良い点:
推奨アクションマージ前に対応すべき項目:
次回以降で対応推奨:
CI は Green です。セキュリティ上の懸念点を修正後、マージ可能と判断します。 詳細なレビューコメントは上記の通りです。お疲れさまでした！ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
Makefile (1)
71-78: DRY principle: Extract duplicated health check loop.The health check loop is duplicated in both
setup-keycloak(lines 52-59) andstart-all(lines 71-78) targets. Consider extracting this into a shared helper function or Makefile variable to reduce maintenance burden.Example refactor using a Makefile variable:
# At top of file after .PHONY declaration KEYCLOAK_HEALTH_CHECK = bash -c 'for i in {1..30}; do \ if curl -s -f http://localhost:8080/health/ready > /dev/null 2>&1; then \ echo "✅ Keycloak が起動しました"; \ break; \ fi; \ echo " 試行 $$i/30..."; \ sleep 2; \ done' # Then use in both targets: @echo "⏳ Keycloak の起動を待っています（最大60秒）..." @$(KEYCLOAK_HEALTH_CHECK)infrastructure/docker/keycloak/scripts/setup-keycloak.sh (2)
137-165: Add clarifying comment: default credentials are for local development only.The hardcoded credentials (username "user", password "password") and admin password defaults are appropriate for local development, but should be documented clearly in the script to prevent accidental use in production contexts.
Add a comment at the top of the user creation section:
echo "" +# Note: The following credentials are for LOCAL DEVELOPMENT ONLY. +# Do not use these defaults in production environments. # デフォルトユーザーを作成 echo "👤 デフォルトユーザー 'user' を作成しています..."Also consider adding a note in the console output to emphasize this:
echo "✅ ユーザー 'user' (パスワード: password) を作成しました"becomes:
echo "✅ ユーザー 'user' (パスワード: password) を作成しました 🔐 (ローカル開発用です)"
139-140: Optional: Use jq for robust JSON parsing instead of grep/sed.The JSON parsing approach (grep/sed) is fragile and could break if the Keycloak API response format changes. While this maintains consistency with existing code in the script (lines 89-90, 116-117), consider migrating to
jqfor robustness.Example refactor:
USER_ID=$(curl -s "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/users?username=user" \ - -H "Authorization: Bearer ${ACCESS_TOKEN}" | grep -o '"id":"[^"]*' | head -1 | sed 's/"id":"//') + -H "Authorization: Bearer ${ACCESS_TOKEN}" | jq -r '.[] | select(.username == "user") | .id' 2>/dev/null)This approach:
- Handles JSON safely
- Is more readable
- Handles edge cases (escaped characters, field ordering)
Note: This would require
jqto be installed in the Keycloak container or the script environment. You may want to address all parsing instances together (lines 89-90, 116-117, 139-140) in a future refactoring effort.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Makefile(1 hunks)infrastructure/docker/keycloak/docker-compose.yml(1 hunks)infrastructure/docker/keycloak/scripts/setup-keycloak.sh(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
🔇 Additional comments (4)
Makefile (2)
48-61: Verify health check interval alignment with PR objectives.The setup-keycloak target implements a 60-second max wait with 2-second intervals (30 iterations × 2s), but the PR objectives state "5秒間隔" (5-second intervals). Please confirm whether the 2-second interval is intentional or should be adjusted to match the documented 5-second intervals.
If 5-second intervals are intended, the loop should be
for i in {1..12}(12 iterations × 5s ≈ 60s max):- @bash -c 'for i in {1..30}; do \ + @bash -c 'for i in {1..12}; do \ if curl -s -f http://localhost:8080/health/ready > /dev/null 2>&1; then \ echo "✅ Keycloak が起動しました"; \ break; \ fi; \ echo " 試行 $$i/30..."; \ - sleep 2; \ + sleep 5; \ done'
36-37: Original review comment is incorrect; no target shadowing occurs.In GNU Make, explicit rules defined later override earlier implicit or pattern rules for the same target. The explicit
dev:target at lines 36-37 takes precedence over the expansion of$(NPM_RUN_TARGETS):at lines 9-10. Whenmake devis run, it will correctly executecd frontend/control-plane && bun run dev, not the root-levelbun run devfrom the pattern rule. The help text at lines 93, 98, and 203 also confirms this is the intended behavior.Likely an incorrect or invalid review comment.
infrastructure/docker/keycloak/docker-compose.yml (1)
37-41: Good alignment with Makefile health check requirements.The addition of
KC_HOSTNAME_URLand health/metrics endpoint enables properly supports the improved health check logic in the Makefile. The configuration changes are appropriate for local development.infrastructure/docker/keycloak/scripts/setup-keycloak.sh (1)
1-20: Idempotency and structure improvements are well-executed.The addition of user creation logic with existence checks (lines 137-165) follows the same idempotent pattern as the existing realm and client checks. The script properly validates resource existence before creation and logs appropriately.
Also applies to: 167-186
PR Review: Keycloak セットアップスクリプトの改善とコマンド追加📊 全体評価このPRは、Keycloakのセットアップスクリプトとビルドフローの両方を改善する内容です。CLAUDE.mdのプレイブックに従い、以下の観点からレビューします。 ✅ 良い点1. 冪等性の向上 🎯
2. ヘルスチェックの改善 💊
3. Plan.md の記録 📝
4. 開発者体験の向上 🚀
|
1 participant
概要
Keycloak のセットアップスクリプトを改善し、Makefile に新しいコマンドを追加しました。
変更内容
keycloak-setupとkeycloak-importコマンドを追加テスト
make keycloak-startでコンテナが正常に起動make keycloak-setupでレルムとクライアントが作成される動作確認
技術的な改善点
Summary by CodeRabbit
Chores
New Features
✏️ Tip: You can customize this high-level summary in your review settings.