Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 141 additions & 0 deletions SilKit/source/config/Test_YamlParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -641,4 +641,145 @@ TEST_F(Test_YamlParser, yaml_throw_on_missing_keyword) {
SilKit::ConfigurationError);
}

TEST_F(Test_YamlParser, yaml_throw_on_healthcheck_as_sequence)
{
// HealthCheck is a mapping; the leading "- " mistypes it as a single-element list.
auto healthCheckAsSequence = R"(
HealthCheck:
- SoftResponseTimeout: 1
)";

EXPECT_THROW(
{
auto cfg = Deserialize<ParticipantConfiguration>(healthCheckAsSequence);
},
SilKit::ConfigurationError);
}

TEST_F(Test_YamlParser, yaml_throw_on_object_as_sequence)
{
// A mapping mistyped as a sequence must be rejected at every nesting level.
const std::initializer_list<const char*> mistypedConfigurations = {
R"(
HealthCheck:
- SoftResponseTimeout: 1
)",
R"(
Logging:
- FlushLevel: Critical
)",
R"(
Middleware:
- RegistryUri: silkit://localhost:8500
)",
R"(
Experimental:
- Metrics:
CollectFromRemote: true
)",
R"(
CanControllers:
- Name: CAN1
Replay:
- UseTraceSource: Source1
)",
R"(
Experimental:
Metrics:
- CollectFromRemote: true
)",
};

for (const auto configuration : mistypedConfigurations)
{
EXPECT_THROW(
{
auto&& cfg = Deserialize<ParticipantConfiguration>(configuration);
(void)cfg;
},
SilKit::ConfigurationError);
}
}

TEST_F(Test_YamlParser, yaml_throw_on_stream_document_as_sequence)
{
// A single "---" document that is itself a bare sequence is the same mistype.
auto streamDocAsSequence = R"(---
- SoftResponseTimeout: 1
)";

EXPECT_THROW(
{
auto&& cfg = Deserialize<ParticipantConfiguration>(streamDocAsSequence);
(void)cfg;
},
SilKit::ConfigurationError);
}

TEST_F(Test_YamlParser, yaml_throw_on_multi_document_stream)
{
// A configuration must be a single YAML document; two "---" documents are rejected.
auto multiDocument = R"(---
ParticipantName: Node0
---
ParticipantName: Node1
)";

EXPECT_THROW(
{
auto&& cfg = Deserialize<ParticipantConfiguration>(multiDocument);
(void)cfg;
},
SilKit::ConfigurationError);
}

TEST_F(Test_YamlParser, yaml_healthcheck_as_map_is_accepted)
{
// Regression guard: the correctly-typed object must still parse.
auto config = Deserialize<ParticipantConfiguration>(R"(
HealthCheck:
SoftResponseTimeout: 1
HardResponseTimeout: 2
)");

ASSERT_TRUE(config.healthCheck.softResponseTimeout.has_value());
ASSERT_TRUE(config.healthCheck.hardResponseTimeout.has_value());
EXPECT_EQ(config.healthCheck.softResponseTimeout.value(), 1ms);
EXPECT_EQ(config.healthCheck.hardResponseTimeout.value(), 2ms);
}

TEST_F(Test_YamlParser, yaml_empty_object_keeps_defaults)
{
// An empty object (null node) keeps its defaults and must not throw.
ParticipantConfiguration defaults{};

auto config = Deserialize<ParticipantConfiguration>(R"(
HealthCheck:
)");

EXPECT_EQ(config.healthCheck.softResponseTimeout, defaults.healthCheck.softResponseTimeout);
EXPECT_EQ(config.healthCheck.hardResponseTimeout, defaults.healthCheck.hardResponseTimeout);
}

TEST_F(Test_YamlParser, yaml_sequence_typed_fields_still_parse)
{
// Genuinely sequence-typed fields (lists of objects) must keep working.
auto config = Deserialize<ParticipantConfiguration>(R"(
CanControllers:
- Name: CAN1
Network: CAN2
- Name: CAN2
LinControllers:
- Name: LIN1
)");

ASSERT_EQ(config.canControllers.size(), 2u);
EXPECT_EQ(config.canControllers.at(0).name, "CAN1");
ASSERT_TRUE(config.canControllers.at(0).network.has_value());
EXPECT_EQ(config.canControllers.at(0).network.value(), "CAN2");
EXPECT_EQ(config.canControllers.at(1).name, "CAN2");
ASSERT_EQ(config.linControllers.size(), 1u);
EXPECT_EQ(config.linControllers.at(0).name, "LIN1");
}

} // anonymous namespace
32 changes: 32 additions & 0 deletions SilKit/source/config/Test_YamlValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,38 @@ TEST_F(Test_YamlValidator, validate_without_warnings)
EXPECT_TRUE(warnings.empty()) << "Warnings: " << warnings;
}

TEST_F(Test_YamlValidator, validate_rejects_multi_document_stream)
{
// A stray "---" mid-config splits it into two documents; the validator must reject it.
auto yamlString = R"yaml(Description: Log to Stdout with Level Info
Logging:
---
Sinks:
- Level: Info
Type: Stdout
)yaml";

std::stringstream warnings;
bool yamlValid = ValidateWithSchema(yamlString, warnings);
EXPECT_FALSE(yamlValid) << "A multi-document stream must not validate";
EXPECT_THAT(warnings.str(), testing::HasSubstr("one YAML document"));
}

TEST_F(Test_YamlValidator, validate_accepts_single_leading_document_marker)
{
// A single leading "---" is one document and stays valid.
auto yamlString = R"yaml(---
schemaVersion: 1
ParticipantName: CanDemoParticipant
)yaml";

std::stringstream warnings;
bool yamlValid = ValidateWithSchema(yamlString, warnings);
EXPECT_TRUE(yamlValid) << "A single leading '---' document must validate. Warnings: "
<< warnings.str();
EXPECT_TRUE(warnings.str().empty()) << "Warnings: " << warnings.str();
}

TEST_F(Test_YamlValidator, validate_unknown_toplevel)
{
auto yamlString = R"yaml(
Expand Down
2 changes: 2 additions & 0 deletions SilKit/source/config/YamlParser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ auto Deserialize(const std::string& input) -> T
// Extract a reference to the root node of the document tree.
auto root = tree.crootref();

VSilKit::EnsureSingleDocument(root);

R reader{parser, root};
T result{};
reader.Read(result);
Expand Down
10 changes: 10 additions & 0 deletions SilKit/source/config/YamlParserUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ auto GetRapidyamlCallbacks() -> ryml::Callbacks
return ryml::Callbacks{nullptr, RapidyamlAllocate, RapidyamlFree, RapidyamlError};
}

void EnsureSingleDocument(ryml::ConstNodeRef root)
{
if (root.is_stream() && root.num_children() > 1)
{
throw SilKit::ConfigurationError{
"configuration must contain exactly one YAML document, but multiple "
"\"---\"-separated documents were found"};
}
}

} // namespace VSilKit


Expand Down
3 changes: 3 additions & 0 deletions SilKit/source/config/YamlParserUtils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,7 @@ auto MakeConfigurationError(ryml::Location location, const std::string_view mess

auto GetRapidyamlCallbacks() -> ryml::Callbacks;

// Throws unless the configuration is a single YAML document.
void EnsureSingleDocument(ryml::ConstNodeRef root);

} // namespace VSilKit
34 changes: 29 additions & 5 deletions SilKit/source/config/YamlReader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,25 +227,49 @@ class BasicYamlReader

auto GetChildSafe(const std::string& name) const -> Impl
{
if (HasKey(name))
if (IsMap())
{
return MakeImpl(_node.find_child(ryml::to_csubstr(name)));
}

if (IsSequence())
if (_node.is_stream())
Comment thread
MariusBgm marked this conversation as resolved.
{
for (const auto& child : _node.cchildren())
// Single-document stream (leading "---"): descend into the document. A
// document that is a bare sequence is the same mistype rejected below.
for (const auto& doc : _node.cchildren())
Comment thread
VDanielEdwards marked this conversation as resolved.
{
if (child.is_container() && HasKey(child, name))
if (doc.is_seq())
{
ThrowExpectedMapping(name);
}
if (doc.is_map())
{
return MakeImpl(child.find_child(ryml::to_csubstr(name)));
auto&& found = doc.find_child(ryml::to_csubstr(name));
if (!found.invalid())
{
return MakeImpl(found);
}
}
}
return MakeImpl({});
}

if (IsSequence())
{
ThrowExpectedMapping(name);
}

// Absent, null or empty node: treat the key as not present.
return MakeImpl({});
}

[[noreturn]] void ThrowExpectedMapping(const std::string& name) const
{
std::ostringstream s;
s << "expected a mapping with key \"" << name << "\", but got a sequence";
throw MakeConfigurationError(s.str());
}

auto MakeImpl(ryml::ConstNodeRef node_) const -> Impl
{
return Impl{_parser, node_};
Expand Down
3 changes: 3 additions & 0 deletions SilKit/source/config/YamlValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,9 @@ bool ValidateWithSchema(const std::string& yamlString, std::ostream& warnings)
auto tree = ryml::parse_in_arena(&parser, cinput);

auto root = tree.crootref();

VSilKit::EnsureSingleDocument(root);

if (root.is_doc() && (root.is_map() || root.is_seq()))
{
std::string version;
Expand Down
7 changes: 6 additions & 1 deletion docs/changelog/versions/latest.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,9 @@

- `config`: default format of file logging is set to JSON. Set "Format: Simple" in the sink to get previous behaviour.
- `docs`: added description of the logging "Format".
- `orchestration`: setting a simulation step duration of zero now raises a `SilKitError` instead of being silently accepted.

## Fixed

- `config`: a mapping that is mistyped as a YAML sequence (e.g. a stray leading `- ` turning `HealthCheck` into a list) is now rejected with a clear error instead of being silently accepted.
- `config`: a configuration containing multiple `---`-separated YAML documents is now rejected with a clear error. A single leading `---` remains valid.
- `orchestration`: setting a simulation step duration of zero now raises a `SilKitError` instead of being silently accepted.
12 changes: 10 additions & 2 deletions docs/configuration/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,17 @@ This gives users the ability to run a simulation with multiple instances of a pa

Many IDEs automatically support participant configuration schema support when the participant configuration file ends with the suffix ``.silkit.json/yaml``.

A participant configuration file is written in YAML syntax according to a specified schema.
It starts with the ``SchemaVersion``, the ``Description`` for the configuration and the ``ParticipantName``.
A participant configuration file is written in YAML syntax according to a specified schema.
It starts with the ``SchemaVersion``, the ``Description`` for the configuration and the ``ParticipantName``.
This is followed by further sections for ``Includes``, ``Middleware``, ``Logging``, ``HealthCheck``, ``Tracing``, ``Extentions`` and sections for the different services of the |ProductName|.

.. admonition:: Note

A configuration must be a single YAML document.
A single leading document marker (``---``) is allowed, but a configuration that
contains multiple ``---``-separated documents is rejected with an error.
This most commonly happens when a stray ``---`` is placed in the middle of a file.

The outline of a participant configuration file is as follows:

.. code-block:: yaml
Expand Down
Loading