Skip to content

MINIFICPP-466 Implement enforcement of dependent property validation #342

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.

Sign up for GitHub

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

Closed
ai-christianson wants to merge 1 commit into from
Closed

MINIFICPP-466 Implement enforcement of dependent property validation #342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 23 additions & 0 deletions libminifi/include/core/yaml/YamlConfiguration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,18 @@ class YamlConfiguration : public FlowConfiguration {
return getYamlRoot(&rootYamlNode);
}

/**
* Iterates all component property validation rules and checks that configured state
* is valid. If state is determined to be invalid, conf parsing ends and an error is raised.
*
* @param component
* @param component_name
* @param yaml_section
*/
void validateComponentProperties(const std::shared_ptr<ConfigurableComponent> &component,
const std::string &component_name,
const std::string &yaml_section) const;

protected:

/**
Expand Down Expand Up @@ -312,6 +324,17 @@ class YamlConfiguration : public FlowConfiguration {
private:
std::shared_ptr<logging::Logger> logger_;
static std::shared_ptr<utils::IdGenerator> id_generator_;

/**
* Raises a human-readable configuration error for the given configuration component/section.
*
* @param component_name
* @param yaml_section
* @param reason
*/
void raiseComponentError(const std::string &component_name,
const std::string &yaml_section,
const std::string &reason) const;
};

} /* namespace core */
Expand Down
59 changes: 44 additions & 15 deletions libminifi/src/core/yaml/YamlConfiguration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,29 +782,58 @@ void YamlConfiguration::parsePropertiesNodeYaml(YAML::Node *propertiesNode,
}
}

validateComponentProperties(processor, component_name, yaml_section);
}

void YamlConfiguration::validateComponentProperties(const std::shared_ptr<ConfigurableComponent> &component,
const std::string &component_name,
const std::string &yaml_section) const {
const auto &component_properties = component->getProperties();

// Validate required properties
for (const auto &prop_pair : processor->getProperties()) {
for (const auto &prop_pair : component_properties) {
if (prop_pair.second.getRequired()) {
const auto &val = prop_pair.second.getValue();

if (val.empty()) {
// Build a helpful error message for the user so they can fix the
// invalid YAML config file, using the component name if present
std::string err_msg =
"Unable to parse configuration file for component named '"
+ component_name
+ "' because required property '" + prop_pair.second.getName() + "' is not set";
if (!yaml_section.empty()) {
err_msg += " [in '" + yaml_section + "' section of configuration file]";
}
logging::LOG_ERROR(logger_) << err_msg;
if (prop_pair.second.getValue().empty()) {
std::string reason("required property '");
reason.append(prop_pair.second.getName());
reason.append("' is not set");
raiseComponentError(component_name, yaml_section, reason);
}
}
}

throw std::invalid_argument(err_msg);
// Validate dependent properties
for (const auto &prop_pair : component_properties) {
const auto &dep_props = prop_pair.second.getDependentProperties();

for (const auto &dep_prop_key : dep_props) {
if (component_properties.at(dep_prop_key).getValue().empty()) {
std::string reason("property '");
reason.append(prop_pair.second.getName());
reason.append("' depends on property '");
reason.append(dep_prop_key);
reason.append("' which is not set");
raiseComponentError(component_name, yaml_section, reason);
}
}
}
}

void YamlConfiguration::raiseComponentError(const std::string &component_name,
const std::string &yaml_section,
const std::string &reason) const {
std::string err_msg = "Unable to parse configuration file for component named '";
err_msg.append(component_name);
err_msg.append("' because " + reason);
if (!yaml_section.empty()) {
err_msg.append(" [in '" + yaml_section + "' section of configuration file]");
}

logging::LOG_ERROR(logger_) << err_msg;

throw std::invalid_argument(err_msg);
}

std::string YamlConfiguration::getOrGenerateId(YAML::Node *yamlNode, const std::string &idField) {
std::string id;
YAML::Node node = yamlNode->as<YAML::Node>();
Expand Down
92 changes: 76 additions & 16 deletions libminifi/test/unit/YamlConfigurationTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ TEST_CASE("Test YAML Config Processing", "[YamlConfiguration]") {
std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration);
std::shared_ptr<core::ContentRepository>
content_repo = std::make_shared<core::repository::VolatileContentRepository>();
core::YamlConfiguration *yamlConfig =
new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);
core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);

SECTION("loading YAML without optional component IDs works") {
static const std::string CONFIG_YAML_WITHOUT_IDS = ""
Expand Down Expand Up @@ -139,7 +138,7 @@ TEST_CASE("Test YAML Config Processing", "[YamlConfiguration]") {
" batch size: 1000";

std::istringstream configYamlStream(CONFIG_YAML_WITHOUT_IDS);
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream);
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream);

REQUIRE(rootFlowConfig);
REQUIRE(rootFlowConfig->findProcessor("TailFile"));
Expand Down Expand Up @@ -187,7 +186,7 @@ TEST_CASE("Test YAML Config Processing", "[YamlConfiguration]") {
"\n";

std::istringstream configYamlStream(CONFIG_YAML_NO_RPG_PORT_ID);
REQUIRE_THROWS_AS(yamlConfig->getYamlRoot(configYamlStream), std::invalid_argument);
REQUIRE_THROWS_AS(yamlConfig.getYamlRoot(configYamlStream), std::invalid_argument);
}
}

Expand All @@ -200,8 +199,7 @@ TEST_CASE("Test YAML v3 Config Processing", "[YamlConfiguration3]") {
std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration);
std::shared_ptr<core::ContentRepository>
content_repo = std::make_shared<core::repository::VolatileContentRepository>();
core::YamlConfiguration *yamlConfig =
new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);
core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);

static const std::string TEST_CONFIG_YAML = R"(
MiNiFi Config Version: 3
Expand Down Expand Up @@ -314,7 +312,7 @@ Remote Process Groups:
NiFi Properties Overrides: {}
)";
std::istringstream configYamlStream(TEST_CONFIG_YAML);
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream);
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream);

REQUIRE(rootFlowConfig);
REQUIRE(rootFlowConfig->findProcessor("TailFile"));
Expand Down Expand Up @@ -353,8 +351,7 @@ TEST_CASE("Test Dynamic Unsupported", "[YamlConfigurationDynamicUnsupported]") {
std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration);
std::shared_ptr<core::ContentRepository>
content_repo = std::make_shared<core::repository::VolatileContentRepository>();
core::YamlConfiguration *yamlConfig =
new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);
core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);

static const std::string TEST_CONFIG_YAML = R"(
Flow Controller:
Expand All @@ -366,7 +363,7 @@ Flow Controller:
Dynamic Property: Bad
)";
std::istringstream configYamlStream(TEST_CONFIG_YAML);
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream);
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream);

REQUIRE(rootFlowConfig);
REQUIRE(rootFlowConfig->findProcessor("PutFile"));
Expand All @@ -391,8 +388,7 @@ TEST_CASE("Test Required Property", "[YamlConfigurationRequiredProperty]") {
std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration);
std::shared_ptr<core::ContentRepository>
content_repo = std::make_shared<core::repository::VolatileContentRepository>();
core::YamlConfiguration *yamlConfig =
new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);
core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);

static const std::string TEST_CONFIG_YAML = R"(
Flow Controller:
Expand All @@ -408,7 +404,7 @@ Flow Controller:
bool caught_exception = false;

try {
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream);
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream);

REQUIRE(rootFlowConfig);
REQUIRE(rootFlowConfig->findProcessor("GetFile"));
Expand Down Expand Up @@ -436,8 +432,7 @@ TEST_CASE("Test Required Property 2", "[YamlConfigurationRequiredProperty2]") {
std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration);
std::shared_ptr<core::ContentRepository>
content_repo = std::make_shared<core::repository::VolatileContentRepository>();
core::YamlConfiguration *yamlConfig =
new core::YamlConfiguration(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);
core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);

static const std::string TEST_CONFIG_YAML = R"(
Flow Controller:
Expand All @@ -450,10 +445,75 @@ Flow Controller:
Batch Size: 1
)";
std::istringstream configYamlStream(TEST_CONFIG_YAML);
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig->getYamlRoot(configYamlStream);
std::unique_ptr<core::ProcessGroup> rootFlowConfig = yamlConfig.getYamlRoot(configYamlStream);

REQUIRE(rootFlowConfig);
REQUIRE(rootFlowConfig->findProcessor("XYZ"));
REQUIRE(NULL != rootFlowConfig->findProcessor("XYZ")->getUUID());
REQUIRE(!rootFlowConfig->findProcessor("XYZ")->getUUIDStr().empty());
}

class DummyComponent : public core::ConfigurableComponent {
public:
bool supportsDynamicProperties() override {
return false;
}

bool canEdit() override {
return true;
}
};

TEST_CASE("Test Dependent Property", "[YamlConfigurationDependentProperty]") {
TestController test_controller;

LogTestController &logTestController = LogTestController::getInstance();
logTestController.setDebug<TestPlan>();
logTestController.setDebug<core::YamlConfiguration>();

std::shared_ptr<core::Repository> testProvRepo = core::createRepository("provenancerepository", true);
std::shared_ptr<core::Repository> testFlowFileRepo = core::createRepository("flowfilerepository", true);
std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration);
std::shared_ptr<core::ContentRepository>
content_repo = std::make_shared<core::repository::VolatileContentRepository>();
core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);
const auto component = std::make_shared<DummyComponent>();
std::set<core::Property> props;
props.emplace(core::Property("Prop A", "Prop A desc", "val A", true, {}, {}));
props.emplace(core::Property("Prop B", "Prop B desc", "val B", true, {"Prop A"}, {}));
component->setSupportedProperties(std::move(props));
yamlConfig.validateComponentProperties(component, "component A", "section A");
REQUIRE(true); // Expected to get here w/o any exceptions
}

TEST_CASE("Test Dependent Property 2", "[YamlConfigurationDependentProperty2]") {
TestController test_controller;

LogTestController &logTestController = LogTestController::getInstance();
logTestController.setDebug<TestPlan>();
logTestController.setDebug<core::YamlConfiguration>();

std::shared_ptr<core::Repository> testProvRepo = core::createRepository("provenancerepository", true);
std::shared_ptr<core::Repository> testFlowFileRepo = core::createRepository("flowfilerepository", true);
std::shared_ptr<minifi::Configure> configuration = std::make_shared<minifi::Configure>();
std::shared_ptr<minifi::io::StreamFactory> streamFactory = minifi::io::StreamFactory::getInstance(configuration);
std::shared_ptr<core::ContentRepository>
content_repo = std::make_shared<core::repository::VolatileContentRepository>();
core::YamlConfiguration yamlConfig(testProvRepo, testFlowFileRepo, content_repo, streamFactory, configuration);
const auto component = std::make_shared<DummyComponent>();
std::set<core::Property> props;
props.emplace(core::Property("Prop A", "Prop A desc", "", false, {}, {}));
props.emplace(core::Property("Prop B", "Prop B desc", "val B", true, {"Prop A"}, {}));
component->setSupportedProperties(std::move(props));
bool config_failed = false;
try {
yamlConfig.validateComponentProperties(component, "component A", "section A");
} catch (const std::exception &e) {
config_failed = true;
REQUIRE("Unable to parse configuration file for component named 'component A' because property "
"'Prop B' depends on property 'Prop A' which is not set "
"[in 'section A' section of configuration file]" == std::string(e.what()));
}
REQUIRE(config_failed);
}