[
https://issues.apache.org/jira/browse/UNOMI-917?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Serge Huber updated UNOMI-917:
------------------------------
Attachment: verify_config_conflicts.py
> Dynamic configuration changes are overridden by custom.system.properties
> because of Karaf's property override system.
> ---------------------------------------------------------------------------------------------------------------------
>
> Key: UNOMI-917
> URL: https://issues.apache.org/jira/browse/UNOMI-917
> Project: Apache Unomi
> Issue Type: Bug
> Components: unomi(-core)
> Affects Versions: unomi-3.0.0, unomi-2.7.0, unomi-3.1.0
> Reporter: Serge Huber
> Priority: Major
> Fix For: unomi-3.2.0, unomi-4.0.0
>
> Attachments: verify_config_conflicts.py
>
>
> h1. Configuration Admin Updates Not Applied Due to Karaf System Property
> Override
> h2. Summary
> Configuration changes made via Karaf's {{config:property-set}} and
> {{config:update}} commands are not being applied because Karaf's
> {{KarafConfigurationPlugin}} automatically overrides Configuration Admin
> properties when matching system properties or environment variables exist.
> h2. Problem Description
> h3. What's Happening
> When administrators try to update configuration properties using Karaf's
> Configuration Admin commands:
> {code:java}
> config:edit org.apache.unomi.router
> config:property-set kafka.host my-kafka-server
> config:update
> {code}
> The changes are saved to the configuration file, but the actual component
> doesn't receive the updated values. Instead, it continues using the values
> from system properties or environment variables.
> h3. Root Cause
> Apache Karaf includes a {{KarafConfigurationPlugin}} that automatically
> overrides Configuration Admin properties when it finds matching system
> properties or environment variables. The override pattern is:
> *System Property Pattern:* {{configurationPid.propertyName}}
> For example:
> * Configuration PID: {{org.apache.unomi.router}}
> * Property name: {{kafka.host}}
> * Karaf looks for: {{org.apache.unomi.router.kafka.host}}
> If this system property exists (either as a system property or environment
> variable), Karaf *always* uses it, completely ignoring any Configuration
> Admin updates.
> h3. Why This Is a Problem
> Unomi uses a large {{custom.system.properties}} file that contains many
> properties following this exact naming convention. These properties were
> originally created to allow environment variable overrides, but they now
> prevent Configuration Admin from working properly.
> *Example:*
> * {{.cfg}} file has: {{{}kafka.host =
> $\{org.apache.unomi.router.kafka.host:-localhost{}}}}
> * {{custom.system.properties}} has:
> {{{}org.apache.unomi.router.kafka.host=$\{env:UNOMI_ROUTER_KAFKA_HOST:-localhost{}}}}
> * Karaf sees the system property {{org.apache.unomi.router.kafka.host}} and
> *always* overrides the Configuration Admin value
> * Result: {{config:property-set}} changes are ignored
> h2. Affected Components
> The following configuration components are affected (excluding
> {{org.apache.unomi.rest.authentication}} which is being fixed separately):
> h3. 1. {{org.apache.unomi.migration}} (1 property)
> * *Property:* {{recoverFromHistory}}
> * *System Property:* {{org.apache.unomi.migration.recoverFromHistory}}
> h3. 2. {{org.apache.unomi.router}} (13 properties)
> * *Properties:* {{{}kafka.host{}}}, {{{}kafka.port{}}},
> {{{}kafka.import.topic{}}}, {{{}kafka.export.topic{}}},
> {{{}kafka.import.groupId{}}}, {{{}kafka.export.groupId{}}},
> {{{}kafka.consumerCount{}}}, {{{}kafka.autoCommit{}}},
> {{{}import.oneshot.uploadDir{}}}, {{{}executionsHistory.size{}}},
> {{{}executions.error.report.size{}}}, {{{}config.allowedEndpoints{}}},
> {{configs.refresh.interval}}
> * *System Properties:* {{org.apache.unomi.router.*}} (all matching the
> property names)
> h3. 3. {{org.apache.unomi.cluster}} (2 properties)
> * *Properties:* {{{}nodeId{}}}, {{nodeStatisticsUpdateFrequency}}
> * *System Properties:* {{{}org.apache.unomi.cluster.nodeId{}}},
> {{org.apache.unomi.cluster.nodeStatisticsUpdateFrequency}}
> h3. 4. {{org.apache.unomi.plugins.base}} (1 property)
> * *Property:* {{maxProfilesInOneMerge}}
> * *System Property:* {{org.apache.unomi.plugins.base.maxProfilesInOneMerge}}
> h3. 5. {{org.apache.unomi.services}} (1 property)
> * *Property:* {{segment.max.retries.update.profile.segment}}
> * *System Property:*
> {{org.apache.unomi.services.segment.max.retries.update.profile.segment}}
> *Total:* 18 properties across 5 components
> h2. Proposed Solution
> h3. Strategy: Rename Properties in {{.cfg}} Files
> Rename the property names in {{.cfg}} files to use domain-specific prefixes
> or hierarchical naming. This breaks the Karaf override pattern while
> maintaining backward compatibility for variable substitution.
> h3. How It Works
> *Karaf Override Pattern:* {{PID.propertyName}} (exact match required)
> *Example for Router:*
> * *Old:* {{.cfg}} has {{{}kafka.host{}}}, system property is
> {{org.apache.unomi.router.kafka.host}} -> *MATCH* -> Override occurs
> * *New:* {{.cfg}} has {{{}router.kafka.host{}}}, system property is
> {{org.apache.unomi.router.kafka.host}} -> Karaf looks for
> {{org.apache.unomi.router.router.kafka.host}} -> *NO MATCH* -> No override
> *Key Point:* The {{{}$\{...{}}}} system property references in {{.cfg}} files
> remain unchanged. They still work for variable substitution, but they no
> longer trigger Karaf's override mechanism.
> h3. Proposed Property Renames
> ||Component||Current Property||Proposed New Name||Rationale||
> |Migration|{{recoverFromHistory}}|{{migration.recoverFromHistory}}|Domain-specific
> prefix|
> |Router|{{kafka.host}}|{{router.kafka.host}}|Add {{router.}} prefix to all
> router properties|
> |Router|{{kafka.port}}|{{router.kafka.port}}|Add {{router.}} prefix to all
> router properties|
> |Router|{{kafka.import.topic}}|{{router.kafka.import.topic}}|Add {{router.}}
> prefix to all router properties|
> |Router|{{kafka.export.topic}}|{{router.kafka.export.topic}}|Add {{router.}}
> prefix to all router properties|
> |Router|{{kafka.import.groupId}}|{{router.kafka.import.groupId}}|Add
> {{router.}} prefix to all router properties|
> |Router|{{kafka.export.groupId}}|{{router.kafka.export.groupId}}|Add
> {{router.}} prefix to all router properties|
> |Router|{{kafka.consumerCount}}|{{router.kafka.consumerCount}}|Add
> {{router.}} prefix to all router properties|
> |Router|{{kafka.autoCommit}}|{{router.kafka.autoCommit}}|Add {{router.}}
> prefix to all router properties|
> |Router|{{import.oneshot.uploadDir}}|{{router.import.oneshot.uploadDir}}|Add
> {{router.}} prefix to all router properties|
> |Router|{{executionsHistory.size}}|{{router.executionsHistory.size}}|Add
> {{router.}} prefix to all router properties|
> |Router|{{executions.error.report.size}}|{{router.executions.error.report.size}}|Add
> {{router.}} prefix to all router properties|
> |Router|{{config.allowedEndpoints}}|{{router.config.allowedEndpoints}}|Add
> {{router.}} prefix to all router properties|
> |Router|{{configs.refresh.interval}}|{{router.configs.refresh.interval}}|Add
> {{router.}} prefix to all router properties|
> |Cluster|{{nodeId}}|{{cluster.nodeId}}|Domain-specific prefix|
> |Cluster|{{nodeStatisticsUpdateFrequency}}|{{cluster.nodeStatisticsUpdateFrequency}}|Domain-specific
> prefix|
> |Plugins
> Base|{{maxProfilesInOneMerge}}|{{merge.maxProfilesInOneMerge}}|Domain-specific
> prefix|
> |Services|{{segment.max.retries.update.profile.segment}}|{{updateProfileSegment.maxRetries}}|More
> readable, domain-specific|
> h3. Implementation Details
> * Update {{.cfg}} files:*
> # Rename property names (left side of {{{}={}}})
> # Keep {{{}$\{...{}}}} system property references unchanged (right side of
> {{{}={}}})
> # Add explanatory comments about why the naming is used
> *Update code references:*
> # Blueprint XML: Update {{<cm:property>}} names and {{{}$\{...{}}}}
> references
> # Java code: Update property lookups and constants
> # Tests: Update property name references
> *System properties:*
> # Old system properties in {{custom.system.properties}} become unused for
> overrides
> # They still work for variable substitution in {{.cfg}} files
> # Can be removed or left for backward compatibility
> h3. Benefits
> * *Fixes the problem:* Configuration Admin updates will work correctly
> * *Backward compatible:* System properties still work for variable
> substitution
> * *No breaking changes:* Users don't need to update
> {{custom.system.properties}}
> * *Better readability:* Some properties become more readable (e.g.,
> {{{}updateProfileSegment.maxRetries{}}})
> * *Self-documenting:* Domain-specific prefixes make configuration clearer
> h3. Impact Assessment
> ||Component||Impact Level||Files to Update||
> |Migration|Low|1 {{{}.cfg{}}}, 1 Java file|
> |Router|High|1 {{{}.cfg{}}}, 1 Blueprint XML (~15 references)|
> |Cluster|Medium|1 {{{}.cfg{}}}, 1 Blueprint XML (2 references)|
> |Plugins Base|Low|1 {{{}.cfg{}}}, 1 Blueprint XML, 1 Java file|
> |Services|Low|1 {{{}.cfg{}}}, 1 Blueprint XML (1 reference)|
> h2. Integration Test Analysis
> h3. Summary
> *Result:* None of the affected properties (excluding V2 compatibility mode)
> are modified dynamically in integration tests using
> {{{}updateConfiguration(){}}}.
> However, one test configuration file uses the old router property names and
> will need to be updated when the renaming is implemented.
> h3. Properties Modified Dynamically in Tests
> h4. 1. V2CompatibilityModeIT.java (Already Fixed)
> * *Properties:* {{{}v2.compatibilitymode.enabled{}}},
> {{v2.compatibilitymode.defaultTenantId}}
> * *Status:* Already updated to use new property names
> * *Impact:* None - already fixed
> h4. 2. RuleServiceIT.java
> * *Property:* {{rules.optimizationActivated}} in
> {{org.apache.unomi.services}}
> * *Status:* NOT affected - this property is not in our conflict list
> * *Impact:* None
> h4. 3. ProfileServiceIT.java
> * *Property:* {{throwExceptions}} in {{org.apache.unomi.persistence.*}}
> * *Status:* NOT affected - this is a persistence property, not in our
> conflict list
> * *Impact:* None
> h3. Properties NOT Modified Dynamically
> The following affected properties are *NOT* modified dynamically in
> integration tests:
> # *Migration:* {{recoverFromHistory}} - No tests modify this
> # *Router:* All 13 properties ({{{}kafka.host{}}}, {{{}kafka.port{}}}, etc.)
> - No tests modify these dynamically
> # *Cluster:* {{{}nodeId{}}}, {{nodeStatisticsUpdateFrequency}} - No tests
> modify these
> # *Plugins Base:* {{maxProfilesInOneMerge}} - No tests modify this
> # *Services:* {{segment.max.retries.update.profile.segment}} - No tests
> modify this
> h3. Test Configuration Files That Need Updates
> h4. Router Test Configuration File
> *File:* {{itests/src/test/resources/org.apache.unomi.router.cfg}}
> *Current properties (OLD names):*
> {code:java}
> router.config.type=nobroker
> import.oneshot.uploadDir=${karaf.data}/tmp/unomi_oneshot_import_configs/
> executionsHistory.size=5
> executions.error.report.size=200
> config.allowedEndpoints=file,ftp,sftp,ftps
> {code}
> *Will need to be updated to (NEW names):*
> {code:java}
> router.config.type=nobroker
> router.import.oneshot.uploadDir=${karaf.data}/tmp/unomi_oneshot_import_configs/
> router.executionsHistory.size=5
> router.executions.error.report.size=200
> router.config.allowedEndpoints=file,ftp,sftp,ftps
> {code}
> *Note:* This file is used in {{BaseIT.java}} line 525:
> {code:java}
> replaceConfigurationFile("etc/org.apache.unomi.router.cfg", new
> File("src/test/resources/org.apache.unomi.router.cfg")),
> {code}
> *Impact:* Low - Only need to update the test configuration file when
> implementing the router property renaming.
> h3. Conclusion
> *Good News:* No integration tests are currently broken by the Karaf override
> issue because:
> * Tests don't dynamically modify the affected properties
> * Tests that do modify properties use properties that are NOT affected
> *Action Required:* When implementing the router property renaming, update the
> test configuration file
> {{itests/src/test/resources/org.apache.unomi.router.cfg}} to use the new
> property names with the {{router.}} prefix.
> h2. Testing Plan
> *Configuration Admin Tests:*
> # Verify {{config:property-set}} and {{config:update}} work for each
> affected component
> # Verify changes are persisted and applied to components
> # Verify {{@Modified}} methods receive updated values
>
> *System Property Tests:*
> # Verify system properties no longer override Configuration Admin
> # Verify system properties still work for variable substitution in {{.cfg}}
> files
> *Integration Tests:*
> # Run existing integration tests
> # Add tests to verify Configuration Admin updates work correctly
> # Update {{itests/src/test/resources/org.apache.unomi.router.cfg}} when
> implementing router property renaming
> h2. Migration Notes
> * *For Users:* No immediate action required. System properties in
> {{custom.system.properties}} will continue to work for variable substitution.
> * *For Administrators:* Configuration Admin commands will now work
> correctly. Old system properties can be removed from
> {{custom.system.properties}} if desired, but it's not required.
> * *Documentation:* Update configuration documentation to reflect new
> property names.
> h2. References
> * Karaf Configuration Documentation: [Environment Variables & System
> Properties|https://karaf.apache.org/manual/latest/#_environment_variables_system_properties]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
