docs(dev): add local development setup guide and IDP scope seed script#985
Open
mulldug wants to merge 1 commit into
Open
docs(dev): add local development setup guide and IDP scope seed script#985mulldug wants to merge 1 commit into
mulldug wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughAdds two new files: ChangesLocal Development Setup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@LOCAL_DEVELOPMENT_HOWTO.md`:
- Line 101: The comment at line 101 in LOCAL_DEVELOPMENT_HOWTO.md hardcodes the
expected resource server ID as 2, which can vary based on setup or preexisting
data and lead to incorrect configurations. Instead of stating "should be 2 for
summit-api," update the documentation to guide developers on how to verify their
actual resource server ID by checking the database or API output, so they use
the correct ID regardless of their setup state.
- Around line 215-217: The IP-ban cleanup instructions in
LOCAL_DEVELOPMENT_HOWTO.md hardcode the IP address 172.18.0.1, which is
environment-specific and may not match the actual blocked IP address. Revise the
troubleshooting step to first query the banned_ips table to identify which IPs
are currently blocked, then provide instructions to delete those returned rows
from both the Redis cache (using redis-cli DEL) and the database (using the
DELETE FROM banned_ips query), replacing the hardcoded IP with a variable or
placeholder that users can substitute with their actual blocked IP address.
- Around line 165-174: The SQL snippet containing the three INSERT statements
(INSERT INTO Member, INSERT INTO Group, and INSERT INTO Group_Members) is not
idempotent and can fail or create duplicates on re-runs. Modify this snippet to
be re-runnable by either: deleting existing test records before the inserts, or
using conditional logic (such as checking if records exist with specific values
like email 'test@test.com' or group code 'super-admins') before inserting, or
using INSERT ... ON DUPLICATE KEY UPDATE statements with appropriate unique
constraints. This ensures the bootstrap process can be safely repeated without
creating duplicates or failing partway through the insertion sequence.
In `@seed_idp_scopes.sql`:
- Around line 29-37: Make the seed script idempotent by converting the INSERT
statements for oauth2_resource_server, oauth2_api, and oauth2_scope tables to
use INSERT ... ON DUPLICATE KEY UPDATE or similar idempotent pattern. At
seed_idp_scopes.sql lines 29-37, update the INSERT statements for
oauth2_resource_server and oauth2_api to be idempotent using appropriate unique
constraints or conditional logic. Apply the same idempotent pattern to the scope
insertions at lines 42-69 and any remaining INSERT statements at lines 74-77 to
ensure the script can be safely re-run without creating duplicate rows or
failing on constraint violations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c20d0e8-8f36-4291-9827-8ec43472705f
📒 Files selected for processing (2)
LOCAL_DEVELOPMENT_HOWTO.mdseed_idp_scopes.sql
| Now link the resource server client from step 3 to the summit-api resource server: | ||
|
|
||
| ```bash | ||
| # Confirm the resource server id (should be 2 for summit-api) |
There was a problem hiding this comment.
Avoid hardcoding expected resource server ID in docs.
“should be 2” is often false once a developer re-runs setup or has preexisting data; this can lead to wrong resource_server_id linkage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LOCAL_DEVELOPMENT_HOWTO.md` at line 101, The comment at line 101 in
LOCAL_DEVELOPMENT_HOWTO.md hardcodes the expected resource server ID as 2, which
can vary based on setup or preexisting data and lead to incorrect
configurations. Instead of stating "should be 2 for summit-api," update the
documentation to guide developers on how to verify their actual resource server
ID by checking the database or API output, so they use the correct ID regardless
of their setup state.
Comment on lines
+165
to
+174
| INSERT INTO Member (ClassName, Created, LastEdited, FirstName, Surname, Email, Active, EmailVerified, ExternalUserIdentifier) | ||
| VALUES ('Member', NOW(), NOW(), 'Test', 'Admin', 'test@test.com', 1, 1, 'test@test.com'); | ||
|
|
||
| SET @member_id = LAST_INSERT_ID(); | ||
|
|
||
| INSERT INTO `Group` (Title, Code) VALUES ('super-admins', 'super-admins'); | ||
| SET @group_id = LAST_INSERT_ID(); | ||
|
|
||
| INSERT INTO Group_Members (GroupID, MemberID) VALUES (@group_id, @member_id); | ||
| EOF |
There was a problem hiding this comment.
Make the test member/group SQL snippet re-runnable.
These inserts are not idempotent. Re-running the guide can fail (or create duplicates) before Group_Members is inserted, breaking repeat local bootstrap.
Suggested direction
-INSERT INTO `Group` (Title, Code) VALUES ('super-admins', 'super-admins');
-SET `@group_id` = LAST_INSERT_ID();
+INSERT INTO `Group` (Title, Code)
+SELECT 'super-admins', 'super-admins'
+WHERE NOT EXISTS (SELECT 1 FROM `Group` WHERE Code = 'super-admins');
+SELECT ID INTO `@group_id` FROM `Group` WHERE Code = 'super-admins' LIMIT 1;📝 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.
Suggested change
| INSERT INTO Member (ClassName, Created, LastEdited, FirstName, Surname, Email, Active, EmailVerified, ExternalUserIdentifier) | |
| VALUES ('Member', NOW(), NOW(), 'Test', 'Admin', 'test@test.com', 1, 1, 'test@test.com'); | |
| SET @member_id = LAST_INSERT_ID(); | |
| INSERT INTO `Group` (Title, Code) VALUES ('super-admins', 'super-admins'); | |
| SET @group_id = LAST_INSERT_ID(); | |
| INSERT INTO Group_Members (GroupID, MemberID) VALUES (@group_id, @member_id); | |
| EOF | |
| INSERT INTO Member (ClassName, Created, LastEdited, FirstName, Surname, Email, Active, EmailVerified, ExternalUserIdentifier) | |
| VALUES ('Member', NOW(), NOW(), 'Test', 'Admin', 'test@test.com', 1, 1, 'test@test.com'); | |
| SET `@member_id` = LAST_INSERT_ID(); | |
| INSERT INTO `Group` (Title, Code) | |
| SELECT 'super-admins', 'super-admins' | |
| WHERE NOT EXISTS (SELECT 1 FROM `Group` WHERE Code = 'super-admins'); | |
| SELECT ID INTO `@group_id` FROM `Group` WHERE Code = 'super-admins' LIMIT 1; | |
| INSERT INTO Group_Members (GroupID, MemberID) VALUES (`@group_id`, `@member_id`); | |
| EOF |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LOCAL_DEVELOPMENT_HOWTO.md` around lines 165 - 174, The SQL snippet
containing the three INSERT statements (INSERT INTO Member, INSERT INTO Group,
and INSERT INTO Group_Members) is not idempotent and can fail or create
duplicates on re-runs. Modify this snippet to be re-runnable by either: deleting
existing test records before the inserts, or using conditional logic (such as
checking if records exist with specific values like email 'test@test.com' or
group code 'super-admins') before inserting, or using INSERT ... ON DUPLICATE
KEY UPDATE statements with appropriate unique constraints. This ensures the
bootstrap process can be safely repeated without creating duplicates or failing
partway through the insertion sequence.
Comment on lines
+215
to
+217
| docker exec openstackid-redis-1 redis-cli -a 1qaz2wsx! DEL "172.18.0.1" 2>/dev/null | ||
| docker exec idp-db-local mysql -u idp_user --password='1qaz2wsx!' idp_local -e "DELETE FROM banned_ips WHERE ip = '172.18.0.1';" | ||
| ``` |
There was a problem hiding this comment.
Generalize IP-ban cleanup instructions.
Using a fixed IP (172.18.0.1) is environment-specific and often won’t match the blocked address. The troubleshooting step should first query banned_ips and delete returned rows.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LOCAL_DEVELOPMENT_HOWTO.md` around lines 215 - 217, The IP-ban cleanup
instructions in LOCAL_DEVELOPMENT_HOWTO.md hardcode the IP address 172.18.0.1,
which is environment-specific and may not match the actual blocked IP address.
Revise the troubleshooting step to first query the banned_ips table to identify
which IPs are currently blocked, then provide instructions to delete those
returned rows from both the Redis cache (using redis-cli DEL) and the database
(using the DELETE FROM banned_ips query), replacing the hardcoded IP with a
variable or placeholder that users can substitute with their actual blocked IP
address.
Comment on lines
+29
to
+37
| INSERT INTO oauth2_resource_server (friendly_name, host, ips, active, created_at, updated_at) | ||
| VALUES ('summit-api', 'localhost', '127.0.0.1', 1, NOW(), NOW()); | ||
|
|
||
| SET @rs_id = LAST_INSERT_ID(); | ||
|
|
||
| INSERT INTO oauth2_api (name, description, active, resource_server_id, created_at, updated_at) | ||
| VALUES ('summit-api', 'Summit API', 1, @rs_id, NOW(), NOW()); | ||
|
|
||
| SET @api_id = LAST_INSERT_ID(); |
There was a problem hiding this comment.
Make the seed script idempotent across resource server/API/scope creation.
Only the client-scope grants are idempotent today. Re-running this script can create duplicate oauth2_resource_server / oauth2_api rows or fail on duplicate scope names, which makes local setup brittle and can drift IDs used downstream.
Suggested direction
-INSERT INTO oauth2_resource_server (friendly_name, host, ips, active, created_at, updated_at)
-VALUES ('summit-api', 'localhost', '127.0.0.1', 1, NOW(), NOW());
-
-SET `@rs_id` = LAST_INSERT_ID();
+INSERT INTO oauth2_resource_server (friendly_name, host, ips, active, created_at, updated_at)
+SELECT 'summit-api', 'localhost', '127.0.0.1', 1, NOW(), NOW()
+WHERE NOT EXISTS (
+ SELECT 1 FROM oauth2_resource_server WHERE friendly_name = 'summit-api'
+);
+SELECT id INTO `@rs_id` FROM oauth2_resource_server WHERE friendly_name = 'summit-api' LIMIT 1;
-INSERT INTO oauth2_api (name, description, active, resource_server_id, created_at, updated_at)
-VALUES ('summit-api', 'Summit API', 1, `@rs_id`, NOW(), NOW());
-
-SET `@api_id` = LAST_INSERT_ID();
+INSERT INTO oauth2_api (name, description, active, resource_server_id, created_at, updated_at)
+SELECT 'summit-api', 'Summit API', 1, `@rs_id`, NOW(), NOW()
+WHERE NOT EXISTS (
+ SELECT 1 FROM oauth2_api WHERE name = 'summit-api'
+);
+SELECT id INTO `@api_id` FROM oauth2_api WHERE name = 'summit-api' LIMIT 1;
-INSERT INTO oauth2_api_scope (...)
+INSERT IGNORE INTO oauth2_api_scope (...)
VALUES (...);Also applies to: 42-69, 74-77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@seed_idp_scopes.sql` around lines 29 - 37, Make the seed script idempotent by
converting the INSERT statements for oauth2_resource_server, oauth2_api, and
oauth2_scope tables to use INSERT ... ON DUPLICATE KEY UPDATE or similar
idempotent pattern. At seed_idp_scopes.sql lines 29-37, update the INSERT
statements for oauth2_resource_server and oauth2_api to be idempotent using
appropriate unique constraints or conditional logic. Apply the same idempotent
pattern to the scope insertions at lines 42-69 and any remaining INSERT
statements at lines 74-77 to ensure the script can be safely re-run without
creating duplicate rows or failing on constraint violations.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Add a guide covering setting up summit-admin connected to local summit-api and openstackid instances.
Add a SQL seed script to assist in setup.
Summary by CodeRabbit