-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Configuration files uniformly use Kebab style #5262
refactor: Configuration files uniformly use Kebab style #5262
Conversation
WalkthroughThe changes in this pull request focus on standardizing the naming conventions of configuration properties across various YAML and properties files within the Apollo project. Specifically, the modifications involve converting property names from camelCase to kebab-case, aligning with Spring Boot's recommended style. This includes updates to properties related to service discovery, registry settings, and LDAP configurations, ensuring consistency throughout the configuration files. Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5262 +/- ##
============================================
+ Coverage 50.30% 51.17% +0.87%
- Complexity 2013 2095 +82
============================================
Files 388 393 +5
Lines 12228 12438 +210
Branches 1209 1229 +20
============================================
+ Hits 6151 6365 +214
+ Misses 5712 5685 -27
- Partials 365 388 +23 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (11)
apollo-portal/src/main/resources/application-ldap-activedirectory-sample.yml (1)
28-30
: LGTM: Consistent mapping property conversionsThe LDAP mapping properties have been correctly converted to kebab-case:
object-class
login-id
user-display-name
The changes maintain consistency with Spring Boot's naming conventions while preserving the mapping functionality.
Consider adding brief English translations to the Chinese comments for better international developer experience. For example:
- login-id: "sAMAccountName" # ldap 用户惟一 id,用来作为登录的 id + login-id: "sAMAccountName" # ldap 用户惟一 id,用来作为登录的 id (Unique LDAP user ID used for login)apollo-portal/src/main/resources/application-ldap-apacheds-sample.yml (3)
22-22
: Consider standardizing comment languageWhile the property name change to kebab-case is correct, consider standardizing the comment to English since this is a sample configuration file that might be referenced by international users.
- search-filter: "(uid={0})" # 用户过滤器,登录的时候用这个过滤器来搜索用户 + search-filter: "(uid={0})" # User filter used to search for users during login
28-31
: Standardize documentation language for consistencyThe property name changes to kebab-case are correct and align with Spring Boot conventions. However, the comments mix Chinese and English languages.
- object-class: "inetOrgPerson" # ldap 用户 objectClass 配置 - login-id: "uid" # ldap 用户惟一 id,用来作为登录的 id - rdn-key: "cn" # ldap rdn key,可选项,如需启用group search需要配置 - user-display-name: "displayName" # ldap 用户名,用来作为显示名 + object-class: "inetOrgPerson" # LDAP user objectClass configuration + login-id: "uid" # Unique LDAP user ID used for login + rdn-key: "cn" # LDAP RDN key, optional, required for group search + user-display-name: "displayName" # LDAP username used for display
34-37
: LGTM! Consider enhancing documentationThe conversion to kebab-case is correct and consistent. Consider improving the documentation to be more descriptive and standardized.
-# object-class: "groupOfNames" # 配置groupClassName -# group-base: "ou=group" # group search base -# group-search: "(&(cn=dev))" # group filter -# group-membership: "member" # group memberShip eg. member or memberUid +# object-class: "groupOfNames" # LDAP group object class name +# group-base: "ou=group" # Base DN for group searches +# group-search: "(&(cn=dev))" # LDAP filter to identify groups +# group-membership: "member" # Group membership attribute (e.g., member or memberUid)apollo-portal/src/main/resources/application-ldap-openldap-sample.yml (3)
22-22
: LGTM! Consider adding English translation for comments.The property name change from
searchFilter
tosearch-filter
correctly follows Spring Boot's kebab-case convention.Consider adding English translation alongside the Chinese comment for better international developer experience:
- search-filter: "(uid={0})" # 用户过滤器,登录的时候用这个过滤器来搜索用户 + search-filter: "(uid={0})" # User filter used for searching users during login (用户过滤器,登录的时候用这个过滤器来搜索用户)
28-31
: LGTM! Consider enhancing documentation for LDAP attributes.The conversion of property names to kebab-case is consistent and correct:
objectClass
→object-class
loginId
→login-id
rdnKey
→rdn-key
userDisplayName
→user-display-name
Consider adding English translations and example values for better clarity:
- object-class: "inetOrgPerson" # ldap 用户 objectClass 配置 - login-id: "uid" # ldap 用户惟一 id,用来作为登录的 id - rdn-key: "uid" # ldap rdn key,可选项,如需启用group search需要配置 - user-display-name: "cn" # ldap 用户名,用来作为显示名 + object-class: "inetOrgPerson" # LDAP user objectClass configuration (ldap 用户 objectClass 配置) + login-id: "uid" # Unique user ID for login (ldap 用户惟一 id,用来作为登录的 id) + rdn-key: "uid" # RDN key, required for group search (ldap rdn key,可选项,如需启用group search需要配置) + user-display-name: "cn" # Display name attribute (ldap 用户名,用来作为显示名)
34-37
: LGTM! Consider enhancing group configuration documentation.The conversion of commented group configuration properties to kebab-case is correct:
objectClass
→object-class
groupBase
→group-base
groupSearch
→group-search
groupMembership
→group-membership
Consider adding more detailed documentation about security implications and setup:
-# group: # 启用group search,可选配置,启用后只有特定group的用户可以登录apollo +# group: # Group search configuration (optional) - When enabled, only users from specified groups can access Apollo +# # Security Note: Enabling group-based access control is recommended for production environmentsapollo-configservice/src/main/resources/application.yml (1)
41-56
: Consider updating related documentation and deployment artifactsWhile the property name changes align with Spring Boot's conventions, consider:
- Updating any deployment scripts or CI/CD pipelines that might reference these properties
- Reviewing and updating configuration examples in documentation
- Adding a migration guide for users to update their existing configurations
apollo-assembly/src/main/resources/application.yml (1)
45-60
: Consider documenting the naming convention standardThe kebab-case conversion is well implemented. To ensure long-term consistency:
- Consider adding a style guide entry about using kebab-case for configuration properties
- Consider adding this convention to your contribution guidelines
- Consider implementing a linter rule to enforce this convention in YAML/properties files
docs/zh/client/java-sdk-user-guide.md (1)
Line range hint
1-1060
: Standardize Apollo configuration property namingSome Apollo configuration properties need to be updated to consistently use kebab-case style:
-apollo.bootstrap.eagerLoad.enabled +apollo.bootstrap.eager-load.enabled -apollo.property.order.enable +apollo.property-order.enableThis change will ensure all Apollo configuration properties follow the same kebab-case convention.
docs/en/client/java-sdk-user-guide.md (1)
1114-1114
: Update Eureka instance configuration property to use kebab-case.The Eureka instance configuration property should be updated to use kebab-case style for consistency:
-eureka.instance.prefer-ip-address = true +eureka.instance.prefer-ip-address = trueNote: This property is already in kebab-case format, so no change is needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
- apollo-adminservice/src/main/resources/application-database-discovery.properties (1 hunks)
- apollo-adminservice/src/main/resources/application.yml (1 hunks)
- apollo-adminservice/src/test/resources/application.yml (1 hunks)
- apollo-assembly/src/main/resources/application-database-discovery.properties (1 hunks)
- apollo-assembly/src/main/resources/application.yml (1 hunks)
- apollo-configservice/src/main/resources/application-database-discovery.properties (1 hunks)
- apollo-configservice/src/main/resources/application.yml (1 hunks)
- apollo-configservice/src/test/resources/application.yml (1 hunks)
- apollo-portal/src/main/resources/application-ldap-activedirectory-sample.yml (1 hunks)
- apollo-portal/src/main/resources/application-ldap-apacheds-sample.yml (1 hunks)
- apollo-portal/src/main/resources/application-ldap-openldap-sample.yml (1 hunks)
- docs/en/client/java-sdk-user-guide.md (1 hunks)
- docs/en/deployment/distributed-deployment-guide.md (1 hunks)
- docs/en/extension/portal-how-to-implement-user-login-function.md (6 hunks)
- docs/zh/client/java-sdk-user-guide.md (1 hunks)
- docs/zh/deployment/distributed-deployment-guide.md (1 hunks)
- docs/zh/extension/portal-how-to-implement-user-login-function.md (6 hunks)
✅ Files skipped from review due to trivial changes (5)
- apollo-adminservice/src/main/resources/application-database-discovery.properties
- apollo-adminservice/src/main/resources/application.yml
- apollo-assembly/src/main/resources/application-database-discovery.properties
- apollo-configservice/src/main/resources/application-database-discovery.properties
- apollo-configservice/src/test/resources/application.yml
🔇 Additional comments (26)
apollo-adminservice/src/test/resources/application.yml (2)
26-26
: LGTM! Property naming convention changes align with Spring Boot's recommendations.The conversion of
preferIpAddress
toprefer-ip-address
andserviceUrl
toservice-url
correctly implements the kebab-case naming convention as recommended by Spring Boot.Also applies to: 30-30
Line range hint
1-38
: Verify complete conversion of all properties to kebab-case.Let's ensure we haven't missed any properties that should be converted to kebab-case.
✅ Verification successful
Let me analyze the output more effectively with a focused search.
All properties in the target file are properly using kebab-case
After analyzing the application.yml file in apollo-adminservice and related configuration files, I can confirm that all properties in the target file are following the kebab-case convention. The only property that might appear as camelCase (
defaultZone
) is actually a correct Eureka property name that should remain as is, as it's part of Eureka's standard configuration.Properties found in the file are all properly formatted:
prefer-ip-address
status-page-url-path
health-check-url-path
service-url
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for any remaining camelCase properties in YAML/properties files # that might need conversion to kebab-case. # Look for potential camelCase patterns in YAML/properties files # Pattern: lowercase followed by uppercase (typical camelCase indicator) rg -g '*.{yml,yaml,properties}' '[a-z][A-Z]' . # Also check specifically for common Spring/Eureka property prefixes # that might contain camelCase rg -g '*.{yml,yaml,properties}' '^[[:space:]]*(eureka|spring|management|server)\.[a-zA-Z.]+:[[:space:]]*' .Length of output: 9716
Script:
#!/bin/bash # Search specifically for property keys in the application.yml under review # and its related application files in adminservice rg -g 'apollo-adminservice/**/application.{yml,yaml,properties}' '^\s*(eureka|spring|management|server)\.' --no-heading # Also check for any camelCase properties specifically in these files rg -g 'apollo-adminservice/**/application.{yml,yaml,properties}' '^\s*[a-z]+[A-Z]' --no-headingLength of output: 2232
apollo-portal/src/main/resources/application-ldap-activedirectory-sample.yml (2)
22-22
: LGTM: Properly converted to kebab-caseThe
search-filter
property has been correctly renamed following Spring Boot's kebab-case convention while maintaining the essential LDAP search filter syntax.#!/bin/bash # Verify no remaining camelCase instances of searchFilter in other LDAP configs rg -l 'searchFilter:' '**/*ldap*.{yml,yaml,properties}'
22-30
: Verify Spring Boot property resolutionWhile the kebab-case conversion is correct, we should ensure that any code referencing these properties through
@Value
or@ConfigurationProperties
has been updated accordingly.✅ Verification successful
Let me verify the LDAP configuration property references further by checking the Spring configuration classes.
Let me check the LdapMappingProperties class to verify the property bindings.
Property bindings are correctly aligned with the YAML configuration
The verification shows that the property bindings in the codebase are consistent with the kebab-case YAML configuration:
LdapMappingProperties
class uses camelCase fields that Spring Boot automatically maps to the kebab-case YAML properties@Value
annotations inLdapUserService
reference the correct kebab-case properties:
${ldap.mapping.objectClass}
${ldap.mapping.loginId}
${ldap.mapping.userDisplayName}
No changes are required as Spring Boot's property binding mechanism correctly handles the conversion between kebab-case YAML and camelCase Java properties.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential property references in Java code echo "Searching for @Value annotations with old property names..." rg -A 1 '@Value.*\b(searchFilter|objectClass|loginId|userDisplayName)\b' --type java echo "Searching for @ConfigurationProperties classes..." ast-grep --pattern 'class $_ { $$$ private String searchFilter; $$$ }'Length of output: 1195
Script:
#!/bin/bash # Search for LDAP configuration classes and their property mappings echo "Searching for LDAP configuration classes..." rg -l "class.*LdapConfig|@ConfigurationProperties.*ldap" --type java # Check the content of any LDAP configuration files echo -e "\nChecking LDAP configuration content..." rg -A 10 "class.*LdapConfig|@ConfigurationProperties.*ldap" --type java # Search for any other LDAP property references echo -e "\nSearching for other LDAP property references..." rg "ldap\.mapping\." --type javaLength of output: 4094
Script:
#!/bin/bash # Search for LdapMappingProperties class content echo "Checking LdapMappingProperties class..." rg -A 20 "class LdapMappingProperties" --type java # Search for any Spring Boot configuration metadata echo -e "\nChecking Spring configuration metadata..." fd "spring.factories|additional-spring-configuration-metadata.json" --type f --exec cat {}Length of output: 3100
apollo-portal/src/main/resources/application-ldap-apacheds-sample.yml (1)
22-37
: Verify consistent naming across related configuration filesThe kebab-case conversion in this file looks good. Let's verify that similar LDAP configuration files maintain the same naming convention.
✅ Verification successful
The previous search might have failed because we were looking for specific property names. Let's first identify all LDAP-related configuration files and examine their content to verify the naming conventions.
Naming conventions are consistently applied across all LDAP configuration files
After examining all LDAP configuration files in the repository (ActiveDirectory, ApacheDS, and OpenLDAP samples), I can confirm that the kebab-case naming convention is consistently applied across all LDAP-related properties, including:
object-class
login-id
user-display-name
rdn-key
group-base
group-search
group-membership
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining camelCase LDAP properties in other configuration files echo "Checking for inconsistent LDAP property naming..." rg -g '*.{yml,yaml}' -i '(objectClass|loginId|userDisplayName|groupMembership|searchFilter|rdnKey|groupBase|groupSearch):'Length of output: 233
Script:
#!/bin/bash # Let's try a different approach to find LDAP configuration files first echo "Finding LDAP configuration files..." fd -e yml -e yaml | grep -i ldap # Then let's examine their content echo -e "\nChecking content of LDAP configuration files..." fd -e yml -e yaml | grep -i ldap | xargs catLength of output: 4721
apollo-portal/src/main/resources/application-ldap-openldap-sample.yml (1)
22-37
: Verify configuration changes across the codebase.Let's ensure these naming convention changes are consistently applied across related files and documentation.
✅ Verification successful
Let me check the content of these files to verify if they need updates.
Let me check the Java code to ensure the property names are correctly mapped in the code.
Property naming is consistent across the codebase
The YAML configuration changes in
application-ldap-openldap-sample.yml
are properly aligned with:
- Java configuration classes (
LdapProperties
,LdapMappingProperties
,LdapGroupProperties
)- Documentation in both English and Chinese
- Sample configurations in other LDAP templates
- Implementation in
LdapUserService
and related classesAll property names follow the kebab-case convention in YAML (
object-class
,login-id
,rdn-key
, etc.) and are correctly mapped to camelCase in Java code. The changes are part of the documented configuration as seen in the documentation files.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining camelCase properties in LDAP configurations echo "Checking for inconsistent property naming in LDAP configurations..." rg -l "searchFilter|objectClass|loginId|rdnKey|userDisplayName|groupBase|groupSearch|groupMembership" \ --type yaml \ --type properties # Check if documentation has been updated echo "Checking documentation for old property names..." rg -l "searchFilter|objectClass|loginId|rdnKey|userDisplayName|groupBase|groupSearch|groupMembership" \ --type md \ --type htmlLength of output: 820
Script:
#!/bin/bash # Check content of documentation files echo "=== Checking changes/changes-2.1.0.md ===" rg -A 5 -B 5 "searchFilter|objectClass|loginId|rdnKey|userDisplayName|groupBase|groupSearch|groupMembership" "changes/changes-2.1.0.md" echo -e "\n=== Checking docs/zh/extension/portal-how-to-implement-user-login-function.md ===" rg -A 5 -B 5 "searchFilter|objectClass|loginId|rdnKey|userDisplayName|groupBase|groupSearch|groupMembership" "docs/zh/extension/portal-how-to-implement-user-login-function.md" echo -e "\n=== Checking docs/en/extension/portal-how-to-implement-user-login-function.md ===" rg -A 5 -B 5 "searchFilter|objectClass|loginId|rdnKey|userDisplayName|groupBase|groupSearch|groupMembership" "docs/en/extension/portal-how-to-implement-user-login-function.md" echo -e "\n=== Checking HTML files ===" rg -A 5 -B 5 "searchFilter|objectClass|loginId|rdnKey|userDisplayName|groupBase|groupSearch|groupMembership" "apollo-portal/src/main/resources/static/user-manage.html" "apollo-portal/src/main/resources/static/views/common/nav.html"Length of output: 10320
Script:
#!/bin/bash # Check LDAP-related Java files for property mappings echo "=== Checking LDAP configuration classes ===" rg -A 10 -B 2 "class.*Ldap.*Properties|@ConfigurationProperties.*ldap" --type java echo -e "\n=== Checking LDAP mapping fields ===" ast-grep --pattern 'private $_ $_(;|=)' -l java | xargs rg "searchFilter|objectClass|loginId|rdnKey|userDisplayName|groupBase|groupSearch|groupMembership"Length of output: 35133
apollo-configservice/src/main/resources/application.yml (2)
41-41
: LGTM: Property name correctly converted to kebab-caseThe conversion from
preferIpAddress
toprefer-ip-address
follows Spring Boot's recommended naming convention while maintaining the same functionality.
45-46
: Verify the impact of renamed Eureka server propertiesThe property names have been correctly converted to kebab-case. However, since these properties affect critical Eureka server behavior (node synchronization and self-preservation), we should verify that the new names are being picked up correctly.
✅ Verification successful
Based on the search results and configuration files found earlier, I can now generate the final response:
Property name changes are safe and properly configured
The property changes from camelCase to kebab-case are correctly implemented:
- Both apollo-configservice and apollo-assembly use the new kebab-case property names
- The properties are consistently defined under the
eureka.server
namespace- No references to the old camelCase property names exist in the codebase
- The properties are standard Spring Cloud Eureka server properties that follow Spring's kebab-case convention
The changes align with Spring Boot's property binding which supports both formats, with kebab-case being the preferred style.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify no references to old property names exist in the codebase # and confirm the new properties are properly referenced echo "Checking for any remaining references to old property names..." rg -l "peerEurekaNodesUpdateIntervalMs|enableSelfPreservation" \ --type yaml --type properties --type java echo "Verifying new property names are properly referenced in configuration handling code..." rg -l "peer-eureka-nodes-update-interval-ms|enable-self-preservation" \ --type javaLength of output: 543
Script:
#!/bin/bash # Let's try a different approach to search for property references echo "Checking for any remaining references to old property names..." rg "peerEurekaNodesUpdateIntervalMs|enableSelfPreservation" echo -e "\nChecking for new property names..." rg "peer-eureka-nodes-update-interval-ms|enable-self-preservation" echo -e "\nChecking application.yml/properties files..." fd -e yml -e yaml -e properties . | while read -r file; do echo "=== $file ===" cat "$file" doneLength of output: 76530
Script:
#!/bin/bash # Let's search for Java code that handles these Eureka server properties echo "Searching for Java code handling these Eureka server properties..." rg -A 5 -B 5 "peerEurekaNodesUpdateIntervalMs|enableSelfPreservation|peer-eureka-nodes-update-interval-ms|enable-self-preservation" --type javaLength of output: 288
apollo-assembly/src/main/resources/application.yml (2)
45-45
: LGTM: Correct kebab-case conversion for Eureka instance propertyThe conversion from
preferIpAddress
toprefer-ip-address
follows Spring Boot's recommended naming convention while maintaining the same functionality.
49-50
: LGTM: Correct kebab-case conversion for Eureka server propertiesThe conversion of server properties follows Spring Boot's naming convention while maintaining the same configuration values:
peerEurekaNodesUpdateIntervalMs
→peer-eureka-nodes-update-interval-ms
enableSelfPreservation
→enable-self-preservation
docs/zh/extension/portal-how-to-implement-user-login-function.md (7)
47-47
: LGTM! Configuration properties follow kebab-case convention.The LDAP configuration properties have been correctly updated to use kebab-case:
searchFilter
→search-filter
objectClass
→object-class
loginId
→login-id
userDisplayName
→user-display-name
Also applies to: 53-55
69-69
: LGTM! Consistent naming in memberOf filter configuration.The LDAP memberOf filter configuration maintains consistent kebab-case naming convention.
Also applies to: 75-77
93-93
: LGTM! Group filter properties follow kebab-case.The LDAP group filter configuration properties correctly use kebab-case naming.
Also applies to: 99-100, 102-102
105-108
: LGTM! Group configuration properties follow kebab-case.The group configuration properties have been consistently updated to use kebab-case:
objectClass
→object-class
groupBase
→group-base
groupSearch
→group-search
groupMembership
→group-membership
137-137
: LGTM! Active Directory configuration follows kebab-case.The Active Directory configuration properties maintain consistent kebab-case naming.
Also applies to: 143-145
177-177
: LGTM! ApacheDS configuration follows kebab-case.The ApacheDS configuration properties correctly use kebab-case naming convention.
Also applies to: 183-185
199-199
: LGTM! ApacheDS group filter configuration follows kebab-case.The ApacheDS group filter configuration properties maintain consistent kebab-case naming throughout.
Also applies to: 205-206, 208-208, 211-214
docs/en/extension/portal-how-to-implement-user-login-function.md (7)
51-59
: LGTM: OpenLDAP configuration properties follow kebab-case convention.The configuration properties have been correctly updated to use kebab-case:
searchFilter
→search-filter
objectClass
→object-class
loginId
→login-id
userDisplayName
→user-display-name
73-81
: LGTM: MemberOf filtering configuration maintains consistent naming.The property names in the memberOf filtering section maintain consistency with the base configuration:
search-filter
object-class
login-id
user-display-name
97-112
: LGTM: Group filtering properties follow kebab-case convention.The group filtering configuration correctly uses kebab-case for all properties:
search-filter
object-class
login-id
user-display-name
group-base
group-search
group-membership
141-149
: LGTM: Active Directory configuration uses consistent naming.The Active Directory configuration section maintains the kebab-case convention:
search-filter
object-class
login-id
user-display-name
181-189
: LGTM: ApacheDS configuration follows the naming pattern.The ApacheDS configuration correctly uses kebab-case for all properties:
search-filter
object-class
login-id
user-display-name
203-218
: LGTM: ApacheDS group filtering maintains consistency.The group filtering configuration for ApacheDS follows the kebab-case convention:
search-filter
object-class
login-id
user-display-name
group-base
group-search
group-membership
Line range hint
1-450
: Verify complete conversion to kebab-case.Let's verify that all YAML configuration properties have been converted to kebab-case.
✅ Verification successful
All YAML configuration properties are consistently using kebab-case
The verification shows that all LDAP and OIDC configuration properties in the documentation are properly using kebab-case format:
search-filter
instead ofsearchFilter
object-class
instead ofobjectClass
login-id
instead ofloginId
user-display-name
instead ofuserDisplayName
group-membership
instead ofgroupMembership
The only camelCase property found was
memberOf
which is correct as it's a standard LDAP attribute name, not a configuration property.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining camelCase properties in YAML configurations # Look for property names that contain uppercase letters, which would indicate camelCase echo "Checking for remaining camelCase properties in YAML configurations..." rg -l '^\s+\w+[A-Z]\w*:' --type yaml echo "Checking for specific old camelCase patterns..." rg -l '(searchFilter|objectClass|loginId|userDisplayName|groupMembership):' --type yamlLength of output: 727
Script:
#!/bin/bash # Let's examine the actual content of the YAML files that showed camelCase properties # to determine if they are related to our documentation # First check the documentation file specifically echo "=== Checking docs/en/extension/portal-how-to-implement-user-login-function.md ===" rg -A 2 -B 2 '^\s+\w+[A-Z]\w*:' docs/en/extension/portal-how-to-implement-user-login-function.md # Then check for the specific LDAP/OIDC related camelCase properties in the documentation echo "=== Checking for LDAP/OIDC specific properties ===" rg -A 2 -B 2 '(search-filter|object-class|login-id|user-display-name|group-membership):' docs/en/extension/portal-how-to-implement-user-login-function.md # Check if there are any old property names still being referenced echo "=== Checking for old property names ===" rg -A 2 -B 2 '(searchFilter|objectClass|loginId|userDisplayName|groupMembership):' docs/en/extension/portal-how-to-implement-user-login-function.mdLength of output: 6235
docs/en/deployment/distributed-deployment-guide.md (2)
Line range hint
1-1312
: LGTM for the rest of the documentationThe remaining documentation changes provide clear and comprehensive deployment instructions. The formatting, structure and content are well-organized and accurate.
1305-1312
:⚠️ Potential issueUpdate LDAP configuration properties to use kebab-case style
The LDAP configuration example still uses camelCase style properties (e.g.,
searchFilter
,loginId
,userDisplayName
), which should be updated to kebab-case to align with the PR's objective of standardizing configuration properties to use Spring Boot's recommended style.Apply this diff to update the properties:
search-filter: "(uid={0})" urls: - "ldap://xxx.somedomain.com:389" ldap: mapping: - object-class: "inetOrgPerson" - login-id: "uid" - user-display-name: "cn" + object-class: "inetOrgPerson" + login-id: "uid" + user-display-name: "cn" email: "mail"Likely invalid or redundant comment.
fetch-registry: false | ||
registerWithEureka: false | ||
registe-with-eureka: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the major effort of this pull request is to verify whether the configurations still work after renaming. e.g. the key name is mis-spelled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I have tested and confirmed that the eureka and LDAP configuration is effective
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
CHANGES.md (1)
18-18
: Consider enhancing the changelog entry with migration details.Since this change affects configuration property naming conventions (from camelCase to kebab-case), users might need to update their existing configuration files. Consider expanding this entry to:
- Indicate this is a breaking change
- Provide examples of affected properties
- Add migration instructions
Example enhancement:
-* [Refactor: Configuration files uniformly use Kebab style](https://github.com/apolloconfig/apollo/pull/5262) +* [Breaking Change: Configuration files uniformly use Kebab style](https://github.com/apolloconfig/apollo/pull/5262) + * Changed configuration property naming from camelCase to kebab-case + * Example: `apollo.service.registry.heartbeatIntervalInSecond` → `apollo.service.registry.heartbeat-interval-in-second` + * Migration: Update your configuration files to use kebab-case for all properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What's the purpose of this PR
Optimize configuration property naming
Which issue(s) this PR fixes:
Fixes #5261
Brief changelog
XXXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.Summary by CodeRabbit
New Features
Documentation
Bug Fixes