Fix: Add missing tenant sync config [master]#10919
Conversation
Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
wso2-engineering-bot
requested review from
chamilaadhi and
tharikaGitHub
as code owners
WalkthroughConfiguration documentation for WSO2 API Manager multitenancy with IS-7 updated to relocate the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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: 1
🧹 Nitpick comments (2)
en/docs/administer/multitenancy/tenant-sharing-with-wso2is7.md (2)
158-165: Clarify TOML structure and tenant sharing context.The configuration example has structural and conceptual clarity issues:
Issues:
Incomplete TOML structure: The configuration shows
[apim.tenant_sharing.properties]without its required parent[[apim.tenant_sharing]]array table declaration. This makes the TOML structure incomplete or potentially invalid.Conceptual confusion: The text states "users can disable tenant sharing and still use the following configuration" (line 157), but the configuration uses
[apim.tenant_sharing.properties]without explicitly showingenable_tenant_sync=falseor explaining why tenant_sharing properties are used when tenant sharing is supposedly disabled.Incomplete example: If demonstrating "orthogonal features" where tenant sync is disabled but auto key manager is enabled, the example should clearly show all necessary configuration, including either omitting the tenant_sharing section entirely (if possible) or showing
enable_tenant_sync=false.💡 Suggested improvements
Option 1: Show complete TOML structure with explicit disable
````toml + [[apim.tenant_sharing]] + type = "WSO2-IS-7" + [apim.tenant_sharing.properties] + enable_tenant_sync=false auto_configure_key_manager=true [apim.key_manager] skip_create_resident_key_manager=true ````Option 2: Clarify if auto_configure_key_manager requires tenant_sharing section
If
auto_configure_key_managercan only be configured under[apim.tenant_sharing.properties], add a note explaining this:Which means users can disable tenant sharing and still use the following configuration to avoid creating default resident key manager and allow creating **WSO2 IS 7.x as a third party key manager** as the default key manager. + !!! note + The `auto_configure_key_manager` property must be set under `[apim.tenant_sharing.properties]` even when tenant synchronization is disabled (by setting `enable_tenant_sync=false` or omitting sync credentials).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@en/docs/administer/multitenancy/tenant-sharing-with-wso2is7.md` around lines 158 - 165, The TOML example is incomplete and confusing: add the missing array table declaration [[apim.tenant_sharing]] and explicitly show enable_tenant_sync=false if you intend to demonstrate tenant sharing being disabled; keep the apim.tenant_sharing.properties block (with auto_configure_key_manager=true) under that array table and retain the apim.key_manager skip_create_resident_key_manager setting, or alternatively remove the tenant_sharing block entirely and note that auto_configure_key_manager must be set under apim.tenant_sharing.properties if that is a requirement—update the example and accompanying text to match whichever option you choose so the structure and intent are unambiguous.
121-127: Clarify TOML configuration structure.The configuration fragment shows
[apim.tenant_sharing.properties]without its parent[[apim.tenant_sharing]]array table declaration. While this might work as an incremental addition to an existing configuration (defined earlier at lines 84-93), it could confuse users about the proper TOML structure.Consider either:
- Adding a note that this assumes the
[[apim.tenant_sharing]]block is already defined (as shown at line 84), or- Showing the complete configuration structure including the parent array declaration
📝 Suggested clarification
Add a note before the configuration block:
For configuring WSO2 IS 7.x as the default key manager you have to add the following configurations to the `<Product-Home>/repository/conf/deployment.toml`: +!!! note + The following configuration should be added to the existing `[[apim.tenant_sharing]]` block configured earlier (lines 84-93). + ````tomlAlternatively, show the complete structure:
-````toml -[apim.tenant_sharing.properties] -auto_configure_key_manager=true - -[apim.key_manager] -skip_create_resident_key_manager=true -```` +````toml +[[apim.tenant_sharing]] +type = "WSO2-IS-7" +[apim.tenant_sharing.properties] +enable_tenant_sync= true +username= "admin" +password= "admin" +identity_server_base_url= "https://localhost:9444" +auto_configure_key_manager=true + +[apim.key_manager] +skip_create_resident_key_manager=true +````🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@en/docs/administer/multitenancy/tenant-sharing-with-wso2is7.md` around lines 121 - 127, The TOML snippet uses [apim.tenant_sharing.properties] without its parent [[apim.tenant_sharing]] array table which may confuse readers; update the doc so the snippet is self-contained by either (A) adding a brief note above the block stating it assumes an existing [[apim.tenant_sharing]] entry defined earlier, or (B) replacing the fragment with the full structure including [[apim.tenant_sharing]] and the nested [apim.tenant_sharing.properties] and [apim.key_manager] entries (reference the headers [[apim.tenant_sharing]], [apim.tenant_sharing.properties], and [apim.key_manager] to locate where to edit).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@en/docs/administer/multitenancy/tenant-sharing-with-wso2is7.md`:
- Around line 136-149: Fix the markdown lint warning by properly
nesting/indenting the fenced TOML block so the admonition recognizes it as a
code block, and clarify the sample scope for apim.tenant_sharing: either (A)
make it a full tenant-sync+key-manager example by adding
apim.tenant_sharing.properties entries enable_tenant_sync, username and password
alongside identity_server_base_url and auto_configure_key_manager, or (B) keep
it as an auto key-manager-only example and add a one-line note stating that
tenant synchronization is disabled/omitted; ensure you update the
apim.key_manager skip_create_resident_key_manager line accordingly and reference
the apim.tenant_sharing, apim.tenant_sharing.properties, enable_tenant_sync,
username, password and apim.key_manager symbols when making the change.
---
Nitpick comments:
In `@en/docs/administer/multitenancy/tenant-sharing-with-wso2is7.md`:
- Around line 158-165: The TOML example is incomplete and confusing: add the
missing array table declaration [[apim.tenant_sharing]] and explicitly show
enable_tenant_sync=false if you intend to demonstrate tenant sharing being
disabled; keep the apim.tenant_sharing.properties block (with
auto_configure_key_manager=true) under that array table and retain the
apim.key_manager skip_create_resident_key_manager setting, or alternatively
remove the tenant_sharing block entirely and note that
auto_configure_key_manager must be set under apim.tenant_sharing.properties if
that is a requirement—update the example and accompanying text to match
whichever option you choose so the structure and intent are unambiguous.
- Around line 121-127: The TOML snippet uses [apim.tenant_sharing.properties]
without its parent [[apim.tenant_sharing]] array table which may confuse
readers; update the doc so the snippet is self-contained by either (A) adding a
brief note above the block stating it assumes an existing
[[apim.tenant_sharing]] entry defined earlier, or (B) replacing the fragment
with the full structure including [[apim.tenant_sharing]] and the nested
[apim.tenant_sharing.properties] and [apim.key_manager] entries (reference the
headers [[apim.tenant_sharing]], [apim.tenant_sharing.properties], and
[apim.key_manager] to locate where to edit).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc4cd985-80d7-43f0-be44-88485bae863e
📒 Files selected for processing (1)
en/docs/administer/multitenancy/tenant-sharing-with-wso2is7.md
| Therefore, in this case sample configuration to register WSO2 IS 7.x as the default key manager would be as follows: | ||
|
|
||
| ````toml | ||
| [[apim.tenant_sharing]] | ||
| type = "WSO2-IS-7" | ||
|
|
||
| [apim.tenant_sharing.properties] | ||
| identity_server_base_url= "https://localhost:9444" | ||
| auto_configure_key_manager=true | ||
|
|
||
| [apim.key_manager] | ||
| skip_create_resident_key_manager = true | ||
| ```` | ||
|
|
There was a problem hiding this comment.
Address markdown linting warning and clarify sample configuration scope.
The static analysis tool flagged a code block style issue at line 136. Additionally, the sample configuration may cause confusion about its intended scope.
Issues identified:
-
Markdown formatting: The fenced code block inside the "important" admonition may need proper indentation for the markdown processor to recognize it correctly.
-
Sample completeness: The configuration shows
[[apim.tenant_sharing]]withtype = "WSO2-IS-7"but omits tenant synchronization properties (enable_tenant_sync,username,password) that were shown in the earlier example (lines 84-93). This creates ambiguity about whether this sample is for:- Full tenant sync + auto key manager configuration, or
- Just auto key manager configuration without tenant sync
🔧 Suggested fixes
Fix 1: Address the markdown linting warning
Ensure the code block is properly formatted within the admonition. The indentation may need adjustment depending on your markdown processor.
Fix 2: Clarify the sample's purpose
Option A - If this is for tenant sync + auto key manager, add the missing properties:
````toml
[[apim.tenant_sharing]]
type = "WSO2-IS-7"
[apim.tenant_sharing.properties]
+ enable_tenant_sync= true
+ username= "admin"
+ password= "admin"
identity_server_base_url= "https://localhost:9444"
auto_configure_key_manager=true
[apim.key_manager]
skip_create_resident_key_manager = true
````Option B - If this is only for auto key manager (no tenant sync), clarify in the text:
- Therefore, in this case sample configuration to register WSO2 IS 7.x as the default key manager would be as follows:
+ Therefore, to register WSO2 IS 7.x as the default key manager (without tenant synchronization), the sample configuration would be as follows:📝 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.
| Therefore, in this case sample configuration to register WSO2 IS 7.x as the default key manager would be as follows: | |
| ````toml | |
| [[apim.tenant_sharing]] | |
| type = "WSO2-IS-7" | |
| [apim.tenant_sharing.properties] | |
| identity_server_base_url= "https://localhost:9444" | |
| auto_configure_key_manager=true | |
| [apim.key_manager] | |
| skip_create_resident_key_manager = true | |
| ```` | |
| Therefore, in this case sample configuration to register WSO2 IS 7.x as the default key manager would be as follows: |
| Therefore, in this case sample configuration to register WSO2 IS 7.x as the default key manager would be as follows: | |
| ````toml | |
| [[apim.tenant_sharing]] | |
| type = "WSO2-IS-7" | |
| [apim.tenant_sharing.properties] | |
| identity_server_base_url= "https://localhost:9444" | |
| auto_configure_key_manager=true | |
| [apim.key_manager] | |
| skip_create_resident_key_manager = true | |
| ```` | |
| Therefore, to register WSO2 IS 7.x as the default key manager (without tenant synchronization), the sample configuration would be as follows: |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 136-136: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@en/docs/administer/multitenancy/tenant-sharing-with-wso2is7.md` around lines
136 - 149, Fix the markdown lint warning by properly nesting/indenting the
fenced TOML block so the admonition recognizes it as a code block, and clarify
the sample scope for apim.tenant_sharing: either (A) make it a full
tenant-sync+key-manager example by adding apim.tenant_sharing.properties entries
enable_tenant_sync, username and password alongside identity_server_base_url and
auto_configure_key_manager, or (B) keep it as an auto key-manager-only example
and add a one-line note stating that tenant synchronization is disabled/omitted;
ensure you update the apim.key_manager skip_create_resident_key_manager line
accordingly and reference the apim.tenant_sharing,
apim.tenant_sharing.properties, enable_tenant_sync, username, password and
apim.key_manager symbols when making the change.
3 participants
This PR was automatically generated by Claude AI.
skip_create_resident_key_managerfrom[apim.tenant_sharing.properties]to[apim.key_manager]section and added complete sample configuration.Summary by CodeRabbit