Skip to content

[20274] Feature/yaml validator v2#543

Open
David-LP99 wants to merge 15 commits into
mainfrom
feature/yaml_validator_v2
Open

[20274] Feature/yaml validator v2#543
David-LP99 wants to merge 15 commits into
mainfrom
feature/yaml_validator_v2

Conversation

@David-LP99

Copy link
Copy Markdown

Removed old python YAML validator and updating the documentation accordingly (still pending a proper update).

Updated ddsrouter_yaml so that when a router configuration is loaded, it has to be validated against the defined JSON schema for the router.

Added tests for the validator and the schema, the examples from the documentation and the resources have to pass the tests, and also a few new test files have to produce the correct result.

Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 9.09091% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 32.34%. Comparing base (1a0f4ad) to head (4e901c6).

Files with missing lines Patch % Lines
ddsrouter_yaml/src/cpp/YamlReaderConfiguration.cpp 11.11% 4 Missing and 12 partials ⚠️
...dsrouter_yaml/src/cpp/YamlReader_configuration.cpp 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #543      +/-   ##
==========================================
+ Coverage   31.14%   32.34%   +1.20%     
==========================================
  Files          16       16              
  Lines         594      575      -19     
  Branches      322      306      -16     
==========================================
+ Hits          185      186       +1     
+ Misses        213      191      -22     
- Partials      196      198       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.com>
Comment on lines +86 to +102
- name: Participant0 # Participant Name = Participant0

kind: local # Participant Kind = local (= simple)

domain: 3 # DomainId = 3

qos:

max-rx-rate: 0 # Max Reception Rate = 0 (unlimited)

downsampling: 1 # Downsampling = 1

####################

# Simple DDS Participant in domain 7

- name: Participant1 # Participant Name = Participant1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same participant name. Also change it in the docu (user_manual/configuration.rst)

2026-07-14 12:52:24.569 [DDSROUTER_ERROR Error] Error Loading DDS Router Configuration from file /home/danny/eProsima/DDS-Pipe/config_docu.yaml. Error message:
 Configuration for DDS Router is invalid: Participant ids are not unique.  -> Function main

routes:
- src: Participant1
dst:
- Participant0 No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Participant0
- Participant0

Comment on lines +73 to +74
"The yaml configuration version "
<< version

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT

Suggested change
"The yaml configuration version "
<< version
"The yaml configuration version " << version

Comment on lines 1079 to 1081
"required":[
"port"
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ddsrouter_yaml/src/cpp/YamlReaderConfiguration.cpp now performs schema validation up front before parsing the router config. That makes the JSON schema part of the mandatory load path, whereas before it was only enforced by the standalone optional validator tool. The problem is that the parser still accepts missing port values and defaults them in ddspipe_yaml/src/cpp/YamlReader_types.cpp (Address::default_port()), so configs that previously loaded successfully can now be rejected earlier by schema validation if an address omits port.

In other words, this PR turns an optional pre-flight schema rule into a runtime hard failure, without aligning it with the parser’s existing defaulting behavior.

For example

With the new file from the docu (ddsrouter_yaml/test/unittest/ddsrouter_yaml_validator/valid_config_files_router/docu_example.yaml), by removing a port in main it does not fail, but with this PR it fails.

I have commented the port variable:

listening-addresses:            # Local Discovery Server Listening Addresses
      - ip: 127.0.0.1               # IP = localhost ; Transport = UDP (by default)
        #port: 11600                 # Port = 11600

And this is the terminal output:

Starting DDS Router Tool execution.
YAML VALIDATION FAILED:

Location: /participants/3/listening-addresses/0
Error: at least one subschema has failed, but all of them are required to validate - required property 'port' not found in object
Value: {
    "ip": "127.0.0.1"
}

Location: /participants/3/listening-addresses/0
Error: [combination: allOf / case#4] required property 'port' not found in object
Value: {
    "ip": "127.0.0.1"
}

2026-07-14 12:59:31.699 [DDSROUTER_ERROR Error] Error Loading DDS Router Configuration from file config_docu.yaml. Error message:
 Error, the provided yaml file is not a valid ddsrouter configuration. -> Function main

Suggested fix:

Align the schema with the parser, or the parser with the schema.

The least disruptive fix (and the one I personally recommend) is to make port optional in resources/configurations/ddsrouter_config_schema.json so it matches the current Address parsing logic.

If the intent is instead to require explicit ports everywhere, then the parser defaulting in YamlReader_types.cpp should be removed and this should be treated as a documented breaking change, with migration notes and regression tests for old configs that omitted port.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

There is already a test for this situation in ddsrouter_yaml/test/unittest/ddsrouter_yaml_validator/invalid_config_files_router/address_no_port.yaml, but I will remove the default value from the parser and update the documentation accordingly.

Comment on lines +77 to +79
###############################################################################
# Resources
###############################################################################

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add this property for debugging purpose. I was changing ddsrouter_config_schema.json but the build files did not refresh.

Suggested change
###############################################################################
# Resources
###############################################################################
###############################################################################
# Resources
###############################################################################
set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS
"${PROJECT_SOURCE_DIR}/../resources/configurations/ddsrouter_config_schema.json")

Comment on lines +80 to +84
throw eprosima::utils::ConfigurationException(
utils::Formatter()
<< "The yaml configuration version " << version
<< " is unknown and not supported. Please update to "
<< ddspipe::yaml::YamlReaderVersion::LATEST << ".");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw eprosima::utils::ConfigurationException(
utils::Formatter()
<< "The yaml configuration version " << version
<< " is unknown and not supported. Please update to "
<< ddspipe::yaml::YamlReaderVersion::LATEST << ".");
// Defensive fallback. with mandatory schema validation enabled, it is not necessary.
throw eprosima::utils::ConfigurationException(utils::Formatter()
<< "The yaml configuration version " << version << " is unknown and not supported." \
"Please update to " << ddspipe::yaml::YamlReaderVersion::LATEST << ".");

@David-LP99 David-LP99 changed the title Feature/yaml validator v2 [20274] Feature/yaml validator v2 Jul 15, 2026
Signed-off-by: David Laseca Perez <davidlaseca@eprosima.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.

2 participants