Read keystore configuration from the toml file#104
Conversation
Change the priority order of reading keystore configs 1.[keystore.intrnal] in deployment.toml 2.[kesytore.primary] in deployment.toml 3.default.json 4.carbon.xml Port of wso2#75 Fixes wso2/product-integrator-mi/issues/4942
📝 WalkthroughOverviewThis PR implements a configuration priority order for the Cipher Tool to read keystore configurations from Key ChangesDependency Updates
Configuration ConstantsExpanded
Configuration Resolution LogicRefactored
New Methods:
Updated Methods:
ImpactUsers can now configure custom keystores in WalkthroughThe cipher tool is updated to read keystore configuration from 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
🤖 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 `@components/ciphertool/pom.xml`:
- Around line 45-48: The com.google.code.gson:gson dependency is declared twice
in the pom.xml file, once at lines 45-48 and again at lines 53-56. Remove one of
these duplicate dependency blocks entirely to eliminate the redundant
declaration. Keep the first or second occurrence (either is fine), and delete
the other complete dependency section including the opening and closing
dependency tags.
In `@components/ciphertool/src/main/java/org/wso2/ciphertool/utils/Utils.java`:
- Around line 344-345: The system property Constants.KEY_LOCATION_PROPERTY is
being set twice in the code: once at line 345 after calling resolveKeyStorePath,
and again later at line 418 via getConfigFilePath, which overwrites the first
assignment. Remove the System.setProperty call for
Constants.KEY_LOCATION_PROPERTY at line 345 (the one immediately following the
resolveKeyStorePath invocation) to eliminate the redundant assignment and avoid
confusion about which value is actually being used.
- Around line 455-460: In the path resolution logic where
path.startsWith("$ref") is checked, add a null safety check after retrieving the
value from defaultMap.get(reference) before calling toString() on it. If the
reference key does not exist in defaultMap and get() returns null, the current
code will throw a NullPointerException. Store the result of
defaultMap.get(reference) in a variable, check if it is null, and either provide
a meaningful error message or handle the missing reference appropriately instead
of directly calling toString() on a potentially null value.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b054017-0d98-40eb-b1c6-db5cdc18461b
📒 Files selected for processing (3)
components/ciphertool/pom.xmlcomponents/ciphertool/src/main/java/org/wso2/ciphertool/utils/Constants.javacomponents/ciphertool/src/main/java/org/wso2/ciphertool/utils/Utils.java
| <dependency> | ||
| <groupId>com.google.code.gson</groupId> | ||
| <artifactId>gson</artifactId> | ||
| </dependency> |
There was a problem hiding this comment.
Duplicate dependency declaration.
The com.google.code.gson:gson dependency is declared twice in this file (also appears at lines 53-56). Remove one of the duplicate entries.
🤖 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 `@components/ciphertool/pom.xml` around lines 45 - 48, The
com.google.code.gson:gson dependency is declared twice in the pom.xml file, once
at lines 45-48 and again at lines 53-56. Remove one of these duplicate
dependency blocks entirely to eliminate the redundant declaration. Keep the
first or second occurrence (either is fine), and delete the other complete
dependency section including the opening and closing dependency tags.
| keyStoreFile = resolveKeyStorePath(keyStoreFile, homeFolder, defaultConfigMap); | ||
| System.setProperty(Constants.KEY_LOCATION_PROPERTY, keyStoreFile); |
There was a problem hiding this comment.
Duplicate system property assignment for KEY_LOCATION_PROPERTY.
Constants.KEY_LOCATION_PROPERTY is set at line 345 (resolved path) and again at line 418 (via getConfigFilePath). The second assignment overwrites the first and may produce a different value. Consider removing line 345 to avoid redundant work and potential inconsistency.
Suggested fix
keyStoreFile = resolveKeyStorePath(keyStoreFile, homeFolder, defaultConfigMap);
- System.setProperty(Constants.KEY_LOCATION_PROPERTY, keyStoreFile);
String keyStoreName = ((Utils.isPrimaryKeyStore()) ? Constants.PRIMARY : Constants.INTERNAL);Also applies to: 417-418
🤖 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 `@components/ciphertool/src/main/java/org/wso2/ciphertool/utils/Utils.java`
around lines 344 - 345, The system property Constants.KEY_LOCATION_PROPERTY is
being set twice in the code: once at line 345 after calling resolveKeyStorePath,
and again later at line 418 via getConfigFilePath, which overwrites the first
assignment. Remove the System.setProperty call for
Constants.KEY_LOCATION_PROPERTY at line 345 (the one immediately following the
resolveKeyStorePath invocation) to eliminate the redundant assignment and avoid
confusion about which value is actually being used.
| if (path.startsWith("$ref")) { | ||
| // Read the value between the curly braces as the reference. | ||
| // e.g. $ref{<reference>} -> <reference> | ||
| String reference = path.substring(path.indexOf('{') + 1, path.indexOf('}')); | ||
| path = defaultMap.get(reference).toString(); | ||
| } |
There was a problem hiding this comment.
Potential NullPointerException when resolving $ref{...} path.
If the reference key extracted from $ref{...} does not exist in defaultMap, line 459 will throw NullPointerException. Consider adding a null check or providing a meaningful error message.
Suggested fix
if (path.startsWith("$ref")) {
// Read the value between the curly braces as the reference.
// e.g. $ref{<reference>} -> <reference>
String reference = path.substring(path.indexOf('{') + 1, path.indexOf('}'));
- path = defaultMap.get(reference).toString();
+ Object refValue = defaultMap.get(reference);
+ if (refValue == null) {
+ throw new CipherToolException("Reference key '" + reference + "' not found in default.json");
+ }
+ path = refValue.toString();
}🤖 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 `@components/ciphertool/src/main/java/org/wso2/ciphertool/utils/Utils.java`
around lines 455 - 460, In the path resolution logic where
path.startsWith("$ref") is checked, add a null safety check after retrieving the
value from defaultMap.get(reference) before calling toString() on it. If the
reference key does not exist in defaultMap and get() returns null, the current
code will throw a NullPointerException. Store the result of
defaultMap.get(reference) in a variable, check if it is null, and either provide
a meaningful error message or handle the missing reference appropriately instead
of directly calling toString() on a potentially null value.
1 participant
Purpose
Port of #75
Fixes wso2/product-integrator-mi/issues/4942