Skip to content
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

Merged

Conversation

youngzil
Copy link
Contributor

@youngzil youngzil commented Oct 26, 2024

What's the purpose of this PR

Optimize configuration property naming

  • Changed camel case naming to hyphen naming to improve readability and consistency
  • Updated configuration properties in multiple files, including Eureka, LDAP and other related configurations
  • Modified the example configurations in Chinese and English documents to reflect the new naming method

Which issue(s) this PR fixes:

Fixes #5261

Brief changelog

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Update the CHANGES log.

Summary by CodeRabbit

  • New Features

    • Introduced a new configuration file for OpenID Connect (OIDC) authentication.
    • Enhanced LDAP authentication with improved filtering capabilities.
    • Added global search functionality for administrators.
  • Documentation

    • Updated user guides for Java SDK and distributed deployment with clearer instructions and new sections.
    • Standardized configuration key naming from camelCase to kebab-case for consistency across documents.
    • Expanded details on Apollo client configuration, including new environment requirements and integration methods.
  • Bug Fixes

    • Corrected various configuration property names in sample YAML files to align with new standards.
    • Resolved issues related to duplicate comments and blank lines in configuration management.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Oct 26, 2024
Copy link
Contributor

coderabbitai bot commented Oct 26, 2024

Walkthrough

The 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

File Path Change Summary
apollo-adminservice/src/main/resources/application-database-discovery.properties Renamed apollo.service.registry.heartbeatIntervalInSecond to apollo.service.registry.heartbeat-interval-in-second.
apollo-adminservice/src/main/resources/application.yml Renamed preferIpAddress to prefer-ip-address, serviceUrl to service-url, and eurekaServiceUrlPollIntervalSeconds to eureka-service-url-poll-interval-seconds.
apollo-adminservice/src/test/resources/application.yml Renamed preferIpAddress to prefer-ip-address and serviceUrl to service-url.
apollo-assembly/src/main/resources/application-database-discovery.properties Renamed apollo.service.registry.heartbeatIntervalInSecond to apollo.service.registry.heartbeat-interval-in-second and apollo.service.discovery.healthCheckIntervalInSecond to apollo.service.discovery.health-check-interval-in-second.
apollo-assembly/src/main/resources/application.yml Renamed preferIpAddress to prefer-ip-address, peerEurekaNodesUpdateIntervalMs to peer-eureka-nodes-update-interval-ms, enableSelfPreservation to enable-self-preservation, serviceUrl to service-url, eurekaServiceUrlPollIntervalSeconds to eureka-service-url-poll-interval-seconds, and registerWithEureka to register-with-eureka.
apollo-configservice/src/main/resources/application-database-discovery.properties Renamed apollo.service.registry.heartbeatIntervalInSecond to apollo.service.registry.heartbeat-interval-in-second and apollo.service.discovery.healthCheckIntervalInSecond to apollo.service.discovery.health-check-interval-in-second.
apollo-configservice/src/main/resources/application.yml Renamed preferIpAddress to prefer-ip-address, peerEurekaNodesUpdateIntervalMs to peer-eureka-nodes-update-interval-ms, enableSelfPreservation to enable-self-preservation, serviceUrl to service-url, and eurekaServiceUrlPollIntervalSeconds to eureka-service-url-poll-interval-seconds.
apollo-portal/src/main/resources/application-ldap-activedirectory-sample.yml Renamed searchFilter to search-filter, objectClass to object-class, loginId to login-id, and userDisplayName to user-display-name.
apollo-portal/src/main/resources/application-ldap-apacheds-sample.yml Renamed searchFilter to search-filter, objectClass to object-class, loginId to login-id, rdnKey to rdn-key, and userDisplayName to user-display-name.
apollo-portal/src/main/resources/application-ldap-openldap-sample.yml Renamed searchFilter to search-filter, objectClass to object-class, loginId to login-id, rdnKey to rdn-key, and userDisplayName to user-display-name.
docs/en/client/java-sdk-user-guide.md Expanded content on Apollo client configuration, including environment requirements and integration with Spring.
docs/en/deployment/distributed-deployment-guide.md Enhanced clarity on deployment instructions for Apollo Configuration Center in distributed environments.
docs/en/extension/portal-how-to-implement-user-login-function.md Updated configuration settings for user authentication methods, aligning with kebab-case conventions.
docs/zh/client/java-sdk-user-guide.md Similar updates as the English version, enhancing clarity and usability for the Apollo client configuration.
docs/zh/deployment/distributed-deployment-guide.md Similar updates as the English version, clarifying deployment instructions for distributed environments.
docs/zh/extension/portal-how-to-implement-user-login-function.md Similar updates as the English version, focusing on user authentication configurations.

Assessment against linked issues

Objective Addressed Explanation
Standardize YAML configuration properties to kebab-case (#[5261])

Suggested labels

size:M, lgtm

Suggested reviewers

  • nobodyiam

🐰 In the fields where rabbits play,
We’ve changed the names in a tidy way.
From camel case to kebab bright,
Configs now look just right!
Hopping through YAML with glee,
Spring Boot’s style, as it should be! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Oct 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.17%. Comparing base (f55b419) to head (d5dc39d).
Report is 14 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 conversions

The 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 language

While 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 consistency

The 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 documentation

The 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 to search-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:

  • objectClassobject-class
  • loginIdlogin-id
  • rdnKeyrdn-key
  • userDisplayNameuser-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:

  • objectClassobject-class
  • groupBasegroup-base
  • groupSearchgroup-search
  • groupMembershipgroup-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 environments
apollo-configservice/src/main/resources/application.yml (1)

41-56: Consider updating related documentation and deployment artifacts

While the property name changes align with Spring Boot's conventions, consider:

  1. Updating any deployment scripts or CI/CD pipelines that might reference these properties
  2. Reviewing and updating configuration examples in documentation
  3. 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 standard

The kebab-case conversion is well implemented. To ensure long-term consistency:

  1. Consider adding a style guide entry about using kebab-case for configuration properties
  2. Consider adding this convention to your contribution guidelines
  3. 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 naming

Some 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.enable

This 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 = true

Note: This property is already in kebab-case format, so no change is needed.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 7ce173e and d5dc39d.

📒 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 to prefer-ip-address and serviceUrl to service-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-heading

Length of output: 2232

apollo-portal/src/main/resources/application-ldap-activedirectory-sample.yml (2)

22-22: LGTM: Properly converted to kebab-case

The 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 resolution

While 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 in LdapUserService 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 java

Length 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 files

The 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 cat

Length 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 classes

All 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 html

Length 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-case

The conversion from preferIpAddress to prefer-ip-address follows Spring Boot's recommended naming convention while maintaining the same functionality.


45-46: Verify the impact of renamed Eureka server properties

The 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 java

Length 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"
done

Length 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 java

Length of output: 288

apollo-assembly/src/main/resources/application.yml (2)

45-45: LGTM: Correct kebab-case conversion for Eureka instance property

The conversion from preferIpAddress to prefer-ip-address follows Spring Boot's recommended naming convention while maintaining the same functionality.


49-50: LGTM: Correct kebab-case conversion for Eureka server properties

The conversion of server properties follows Spring Boot's naming convention while maintaining the same configuration values:

  • peerEurekaNodesUpdateIntervalMspeer-eureka-nodes-update-interval-ms
  • enableSelfPreservationenable-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:

  • searchFiltersearch-filter
  • objectClassobject-class
  • loginIdlogin-id
  • userDisplayNameuser-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:

  • objectClassobject-class
  • groupBasegroup-base
  • groupSearchgroup-search
  • groupMembershipgroup-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:

  • searchFiltersearch-filter
  • objectClassobject-class
  • loginIdlogin-id
  • userDisplayNameuser-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 of searchFilter
  • object-class instead of objectClass
  • login-id instead of loginId
  • user-display-name instead of userDisplayName
  • group-membership instead of groupMembership

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 yaml

Length 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.md

Length of output: 6235

docs/en/deployment/distributed-deployment-guide.md (2)

Line range hint 1-1312: LGTM for the rest of the documentation

The remaining documentation changes provide clear and comprehensive deployment instructions. The formatting, structure and content are well-organized and accurate.


1305-1312: ⚠️ Potential issue

Update 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
Copy link
Member

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.

Copy link
Contributor Author

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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:

  1. Indicate this is a breaking change
  2. Provide examples of affected properties
  3. 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
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 039fa54 and d768748.

📒 Files selected for processing (1)
  • CHANGES.md (1 hunks)

Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 27, 2024
@nobodyiam nobodyiam merged commit d886164 into apolloconfig:master Oct 27, 2024
7 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2024
@youngzil youngzil deleted the feature/configuration-to-kebab-style branch October 27, 2024 16:10
@nobodyiam nobodyiam added this to the 2.4.0 milestone Jan 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Yaml中的配置文件统一使用SpringBoot官方推荐的Kebab风格
2 participants