Skip to content

[improve][admin] Validate dynamic configuration updates against FieldContext constraints#26087

Open
Dream95 wants to merge 1 commit into
apache:masterfrom
Dream95:improve_dynamic_config
Open

[improve][admin] Validate dynamic configuration updates against FieldContext constraints#26087
Dream95 wants to merge 1 commit into
apache:masterfrom
Dream95:improve_dynamic_config

Conversation

@Dream95

@Dream95 Dream95 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Motivation

Most built-in dynamic configuration updates were not checked against @FieldContext before being written to metadata. Validation only ran when a custom Predicate was registered, for example on loadManagerClassName.
That meant invalid strings could be stored and only fail later at runtime, or not fail at all. Boolean fields were a concrete example: FieldParser uses Boolean.valueOf(), so failureDomainsEnabled=abc was stored as "abc" but
applied as false with no error.

Modifications

  • Route isComplete() through FieldContextValidator.validateParsedValue().
  • Validate admin dynamic-config updates with FieldContextValidator.validateString() before persisting to ZK.
  • Add BrokerService.getDynamicConfigurationValidationError() and return the specific message on HTTP 412 instead of " Invalid dynamic-config value".
  • Run custom validators (e.g. classpath check for loadManagerClassName) after FieldContext checks.
  • Add FieldContextValidatorTest and extend AdminApiDynamicConfigurationsTest.

Admin updates now reject boolean values that are not exactly true or false.
Startup and runtime loading still use FieldParser, which calls Boolean.valueOf() and treats any other string as false.
This change is scoped to the admin write path. Tightening broker.conf parsing or ZK replay would be a separate compatibility discussion.

Verifying this change

  • Make sure that the change passes the CI checks.

  • ./gradlew :pulsar-broker-common:test --tests org.apache.pulsar.common.configuration.FieldContextValidatorTest

  • ./gradlew :pulsar-broker-common:test --tests org.apache.pulsar.common.configuration.PulsarConfigurationLoaderTest

  • ./gradlew :pulsar-broker:test --tests org.apache.pulsar.broker.admin.AdminApiDynamicConfigurationsTest

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Behavior changes

Affected admin API:

  • POST /admin/v2/brokers/configuration/{configName}/{configValue}
    Changes on this endpoint:
  • Built-in dynamic configs now reject values that fail type parsing, minValue/maxValue, maxCharLength, or boolean format.
  • HTTP 412 responses include a specific error message in the response body.
  • Unknown keys return Unknown dynamic configuration: <key> instead of Can't update non-dynamic configuration.
  • Boolean dynamic configs accept only true or false (case-insensitive).

…Context constraints

Signed-off-by: Dream95 <zhou_8621@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant