Repository navigation
ci: CI にテスト・型チェック・ビルドを追加 - #85
Conversation
CI ワークフロー: - make typecheck を追加 - make test（カバレッジ付き）を追加 - make build を追加 - install_ci を拡張して全アプリの依存関係をインストール Keycloak / tenant-management-service: - tenant-management-service クライアントを追加 - serviceAccountsEnabled を有効化 - realm-admin 権限を付与 - docker-compose に Keycloak 環境変数を追加 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe pull request updates CI/CD infrastructure and tenant-management provisioning. It adds type checking, testing, and build steps to the CI workflow; refactors the Makefile's install_ci target to iterate over multiple frontend apps and install problem-management dependencies; adds Keycloak environment variables to the tenant-management service; and extends the Keycloak setup script to enable service accounts and grant realm-admin role to the tenant-management-service. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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では、CIワークフローに typecheck、test、build を追加し、make before-commit と同等のチェックを実現しています。また、Keycloak に tenant-management-service クライアントを追加し、realm-admin 権限を付与する実装も含まれています。 ✅ 良い点
🔍 改善提案1. セキュリティ: ハードコードされたシークレット重大度: 高 docker-compose.yml と setup-keycloak.sh で KEYCLOAK_CLIENT_SECRET=secret がハードコードされています。 docker-compose.yml:105
setup-keycloak.sh:138 推奨事項:
2. エラーハンドリング: Keycloak セットアップの失敗処理重大度: 中 setup-keycloak.sh の権限付与セクション（142-174行目）で、エラーが発生した場合のハンドリングが不十分です。 問題点:
推奨事項: 3. Makefile: install_ci の改善重大度: 低 Makefile:54-55 で PROBLEM_MANAGEMENT_DIR のインストールのみを実行していますが、将来的に他のバックエンドサービスが追加された場合、修正が必要になります。 推奨事項: 4. CI ワークフロー: テスト結果の可視化重大度: 低 テストとカバレッジを実行していますが、結果をアーティファクトとして保存していません。 推奨事項:
5. ドキュメント: 変更内容の説明不足重大度: 低 PR 本文に「tenant-management-service クライアントを追加」とありますが、なぜこのクライアントが必要なのか、どのように使用されるのかの説明がありません。 推奨事項: 🧪 テストカバレッジCLAUDE.md によると、カバレッジ 100％ が必須条件ですが、このPRではテストコードの追加がありません。 確認事項:
📋 チェックリスト（CLAUDE.md の 2.7 より）
🎯 総評CI の強化とKeycloak の Service Account 設定は正しい方向性です。ただし、以下の点に対応することを推奨します：
CI が Green になることを確認後、上記の修正を行うことをお勧めします。 🤖 Generated with Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci.yml(1 hunks)Makefile(1 hunks)docker-compose.yml(1 hunks)infrastructure/docker/keycloak/scripts/setup-keycloak.sh(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: susumutomita/TenkaCloud PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T06:19:22.178Z
Learning: テスト実行前に make before-commit コマンドを実行し、lint_text、format_check、typecheck、test、build がすべて成功することを確認する。
📚 Learning: 2025-11-30T06:19:22.178Z
Learnt from: CR
Repo: susumutomita/TenkaCloud PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-30T06:19:22.178Z
Learning: テスト実行前に make before-commit コマンドを実行し、lint_text、format_check、typecheck、test、build がすべて成功することを確認する。
Applied to files:
Makefile.github/workflows/ci.yml
⏰ 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). (2)
- GitHub Check: claude-review
- GitHub Check: ci
🔇 Additional comments (3)
Makefile (1)
50-56: LGTM! Theinstall_cirefactoring correctly iterates over all frontend apps and backend services with proper error handling and supply chain security flags..github/workflows/ci.yml (1)
53-63: LGTM! The three new CI steps (Type check, Run tests, Build) are logically sequenced after Format check and correctly invoke the Makefile targets. This mirrors the localmake before_commitworkflow, ensuring consistency between developer machines and CI. Per learnings, this validates the full CI pipeline as specified.infrastructure/docker/keycloak/scripts/setup-keycloak.sh (1)
116-116: LGTM! AddingserviceAccountsEnabled: truecorrectly enables service accounts for clients, which is necessary for the tenant-management-service to authenticate with Keycloak using credentials.
| - KEYCLOAK_URL=http://keycloak:8080 | ||
| - KEYCLOAK_CLIENT_ID=tenant-management-service | ||
| - KEYCLOAK_CLIENT_SECRET=secret |
There was a problem hiding this comment.
Harden Keycloak credentials against plaintext exposure. The hardcoded KEYCLOAK_CLIENT_SECRET=secret should be environment-variable based, consistent with other services in the compose file (e.g., AUTH_SECRET, AUTH_KEYCLOAK_SECRET). Even for local development, storing credentials as environment variables improves separation of concerns and prevents accidental commits of real secrets.
Apply this diff to externalize the credentials:
environment:
- DATABASE_URL=postgresql://postgres:postgres@postgres:5432/tenkacloud?schema=public
- KEYCLOAK_URL=http://keycloak:8080
- - KEYCLOAK_CLIENT_ID=tenant-management-service
- - KEYCLOAK_CLIENT_SECRET=secret
+ - KEYCLOAK_CLIENT_ID=${KEYCLOAK_CLIENT_ID:-tenant-management-service}
+ - KEYCLOAK_CLIENT_SECRET=${KEYCLOAK_CLIENT_SECRET:-secret}Alternatively, if these values should always be fixed for local development, consider documenting this decision in a comment explaining the security implications.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - KEYCLOAK_URL=http://keycloak:8080 | |
| - KEYCLOAK_CLIENT_ID=tenant-management-service | |
| - KEYCLOAK_CLIENT_SECRET=secret | |
| - KEYCLOAK_URL=http://keycloak:8080 | |
| - KEYCLOAK_CLIENT_ID=${KEYCLOAK_CLIENT_ID:-tenant-management-service} | |
| - KEYCLOAK_CLIENT_SECRET=${KEYCLOAK_CLIENT_SECRET:-secret} |
🤖 Prompt for AI Agents
In docker-compose.yml around lines 103 to 105, the Keycloak client secret is
hardcoded as KEYCLOAK_CLIENT_SECRET=secret; replace it with an
environment-variable reference (for example use the existing
AUTH_KEYCLOAK_SECRET or a new KEYCLOAK_CLIENT_SECRET env var) and optionally
provide a safe local default expansion for development. Update the service's
environment list to pull the secret from the external env (and document the
chosen behavior in a short comment if you decide the value should remain fixed
for local dev) so credentials are not stored in plaintext in the compose file.
| create_client "control-plane-ui" "http://localhost:3000/*" "http://localhost:3000" "secret" | ||
| create_client "admin-app" "http://localhost:3001/*" "http://localhost:3001" "secret" | ||
| create_client "participant-app" "http://localhost:3002/*" "http://localhost:3002" "secret" | ||
| create_client "tenant-management-service" "http://localhost:3004/*" "http://localhost:3004" "secret" |
There was a problem hiding this comment.
Use environment variables for tenant-management credentials. The script now receives KEYCLOAK_CLIENT_ID and KEYCLOAK_CLIENT_SECRET from docker-compose.yml but continues to hardcode "tenant-management-service" and "secret". This creates a security inconsistency and prevents flexible credential management. Leverage the environment variables to align with the broader setup.
Apply this diff to use environment variables:
# クライアント作成 (Secret は 'secret' で統一)
create_client "control-plane-ui" "http://localhost:3000/*" "http://localhost:3000" "secret"
create_client "admin-app" "http://localhost:3001/*" "http://localhost:3001" "secret"
create_client "participant-app" "http://localhost:3002/*" "http://localhost:3002" "secret"
-create_client "tenant-management-service" "http://localhost:3004/*" "http://localhost:3004" "secret"
+TENANT_MGMT_CLIENT_ID="${KEYCLOAK_CLIENT_ID:-tenant-management-service}"
+TENANT_MGMT_CLIENT_SECRET="${KEYCLOAK_CLIENT_SECRET:-secret}"
+create_client "${TENANT_MGMT_CLIENT_ID}" "http://localhost:3004/*" "http://localhost:3004" "${TENANT_MGMT_CLIENT_SECRET}"Then update the admin-grant flow to use the variable:
-CLIENT_UUID=$(curl -s "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients?clientId=tenant-management-service" \
+CLIENT_UUID=$(curl -s "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients?clientId=${TENANT_MGMT_CLIENT_ID}" \
-H "Authorization: Bearer ${ACCESS_TOKEN}" | grep -o '"id":"[^"]*' | head -1 | sed 's/"id":"//')Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
in infrastructure/docker/keycloak/scripts/setup-keycloak.sh around lines
135-138, the client creation call hardcodes the tenant-management service id and
secret; replace the literal "tenant-management-service" and "secret" with the
environment variables KEYCLOAK_CLIENT_ID and KEYCLOAK_CLIENT_SECRET (use the
shell variable expansions already provided by docker-compose) so the client is
created with the configured values, and update the subsequent admin-grant flow
in this script to reference the same KEYCLOAK_CLIENT_ID variable instead of the
hardcoded name so both creation and grant steps use the injected credentials
consistently.
| create_client "control-plane-ui" "http://localhost:3000/*" "http://localhost:3000" "secret" | ||
| create_client "admin-app" "http://localhost:3001/*" "http://localhost:3001" "secret" | ||
| create_client "participant-app" "http://localhost:3002/*" "http://localhost:3002" "secret" | ||
| create_client "tenant-management-service" "http://localhost:3004/*" "http://localhost:3004" "secret" |
There was a problem hiding this comment.
Add error handling and avoid silent failures in the admin-grant flow. The curl operations in lines 145–166 lack error checking. If any network call fails (e.g., malformed response, network timeout), the script continues silently. With set -e only catching non-zero exit codes, failed command substitutions $(...) won't halt execution.
Apply this diff to add explicit error handling:
# Get Client UUID
-CLIENT_UUID=$(curl -s "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients?clientId=tenant-management-service" \
- -H "Authorization: Bearer ${ACCESS_TOKEN}" | grep -o '"id":"[^"]*' | head -1 | sed 's/"id":"//')
+CLIENT_UUID=$(curl -s -f "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients?clientId=tenant-management-service" \
+ -H "Authorization: Bearer ${ACCESS_TOKEN}" 2>/dev/null | grep -o '"id":"[^"]*' | head -1 | sed 's/"id":"//')
if [ -n "$CLIENT_UUID" ]; then
# Get Service Account User ID
- SERVICE_ACCOUNT_USER_ID=$(curl -s "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients/${CLIENT_UUID}/service-account-user" \
- -H "Authorization: Bearer ${ACCESS_TOKEN}" | grep -o '"id":"[^"]*' | head -1 | sed 's/"id":"//')
+ SERVICE_ACCOUNT_USER_ID=$(curl -s -f "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients/${CLIENT_UUID}/service-account-user" \
+ -H "Authorization: Bearer ${ACCESS_TOKEN}" 2>/dev/null | grep -o '"id":"[^"]*' | head -1 | sed 's/"id":"//')
if [ -n "$SERVICE_ACCOUNT_USER_ID" ]; then
# Get realm-management Client UUID
- REALM_MANAGEMENT_ID=$(curl -s "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients?clientId=realm-management" \
- -H "Authorization: Bearer ${ACCESS_TOKEN}" | grep -o '"id":"[^"]*' | head -1 | sed 's/"id":"//')
+ REALM_MANAGEMENT_ID=$(curl -s -f "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients?clientId=realm-management" \
+ -H "Authorization: Bearer ${ACCESS_TOKEN}" 2>/dev/null | grep -o '"id":"[^"]*' | head -1 | sed 's/"id":"//')
# Get realm-admin Role info
- ROLE_INFO=$(curl -s "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients/${REALM_MANAGEMENT_ID}/roles/realm-admin" \
- -H "Authorization: Bearer ${ACCESS_TOKEN}")
+ ROLE_INFO=$(curl -s -f "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/clients/${REALM_MANAGEMENT_ID}/roles/realm-admin" \
+ -H "Authorization: Bearer ${ACCESS_TOKEN}" 2>/dev/null)
# Assign role
- curl -s -X POST "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/users/${SERVICE_ACCOUNT_USER_ID}/role-mappings/clients/${REALM_MANAGEMENT_ID}" \
+ curl -s -f -X POST "${KEYCLOAK_URL}/admin/realms/${REALM_NAME}/users/${SERVICE_ACCOUNT_USER_ID}/role-mappings/clients/${REALM_MANAGEMENT_ID}" \
-H "Authorization: Bearer ${ACCESS_TOKEN}" \
-H "Content-Type: application/json" \
-d "[${ROLE_INFO}]" > /dev/nullThe -f flag causes curl to exit with status 22 on HTTP errors (4xx/5xx), and the script's set -e will then halt execution on failure.
Also applies to: 142-174
🤖 Prompt for AI Agents
in infrastructure/docker/keycloak/scripts/setup-keycloak.sh around lines 138 and
142-174, the admin-grant curl calls currently perform command substitutions
without checking for HTTP or network errors, which can lead to silent failures;
modify each curl invocation to use the -f flag (or test HTTP status explicitly)
and check return values of command substitutions, e.g., capture responses/status
in variables and test for non-empty/expected fields and non-zero curl exit
codes, and fail fast (exit 1) with an explanatory error message when a call
fails so set -e will reliably stop the script on HTTP/network errors.
…(ADR-037 Slice 3) (#1901) Picks up TenkaCloudChallenge #85 (708705f): SCHEMA.json now allows disruptions[].recurrence, so a problem can declare a score-gated recurring disruption past `make validate-problems`. Completes the ADR-037 Slice 3 pairing with the platform engine (PR-1900). Additive; all existing problems still validate. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
概要
CI ワークフローにテスト、型チェック、ビルドを追加し、
make before-commitと同等のチェックを実行するようにした。変更内容
CI ワークフロー
make typecheckを追加（全フロントエンドアプリの型チェック）make testを追加（カバレッジ付きテスト）make buildを追加（ビルド確認）install_ciを拡張して全フロントエンドアプリとバックエンドサービスの依存関係をインストールKeycloak / tenant-management-service
serviceAccountsEnabledを有効化テスト
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.