brbzull0 commented on code in PR #9942:
URL: https://github.com/apache/trafficserver/pull/9942#discussion_r1281783651
##########
plugins/conf_remap/conf_remap.cc:
##########
@@ -122,16 +124,99 @@ RemapConfigs::parse_inline(const char *arg)
return true;
}
-// Config file parser, somewhat borrowed from P_RecCore.i
-bool
-RemapConfigs::parse_file(const char *filename)
+namespace
+{
+TSRecordDataType
+try_deduce_type(YAML::Node node)
+{
+ std::string_view tag = node.Tag();
+ if (tag == ts::Yaml::YAML_FLOAT_TAG_URI) {
+ return TS_RECORDDATATYPE_FLOAT;
+ } else if (tag == ts::Yaml::YAML_INT_TAG_URI) {
+ return TS_RECORDDATATYPE_INT;
+ } else if (tag == ts::Yaml::YAML_STR_TAG_URI) {
+ return TS_RECORDDATATYPE_STRING;
+ }
+ // we only care about string, int and float.
+ return TS_RECORDDATATYPE_NULL;
+}
+/// @brief Context class used to pass information to the TSYAMLRecNodeHandler
as the data obj.
+struct Context {
+ RemapConfigs::Item *items;
+ int *current;
+};
+
+TSReturnCode
+scalar_node_handler(const TSYAMLRecCfgFieldData *cfg, void *data)
{
- int line_num = 0;
- TSFile file;
- char buf[8192];
TSOverridableConfigKey name;
- TSRecordDataType type, expected_type;
+ TSRecordDataType expected_type;
+ std::string text;
+ auto &ctx = *static_cast<Context *>(data);
+ YAML::Node value = *reinterpret_cast<YAML::Node *>(cfg->value_node);
+
+ if (TSHttpTxnConfigFind(cfg->record_name, strlen(cfg->record_name), &name,
&expected_type) != TS_SUCCESS) {
+ TSError("[%s] '%s' is not a configuration variable or cannot be
overridden", PLUGIN_NAME, cfg->record_name);
+ return TS_ERROR;
+ }
+
+ RemapConfigs::Item *item = &ctx.items[*ctx.current];
+
+ auto type = try_deduce_type(value);
+
+ // If we detected a type but it's different from the one registered with the
in ATS, then we ignore it.
+ if (type != TS_RECORDDATATYPE_NULL && expected_type != type) {
+ TSError("%s", swoc::bwprint(text, "[{}] '{}' variable type mismatch,
expected {}, got {}", PLUGIN_NAME, cfg->record_name,
+ static_cast<int>(expected_type),
static_cast<int>(type))
+ .c_str());
+ return TS_ERROR; // Ignore the field
+ }
+
+ // We use the expected type. If no type set or the time did match, then it's
+ // safe to use the expected type.
+ try {
+ switch (expected_type) {
+ case TS_RECORDDATATYPE_INT:
+ item->_data.rec_int = value.as<int64_t>();
+ break;
+ case TS_RECORDDATATYPE_STRING: {
+ std::string str = value.as<std::string>();
+ if (str == "NULL") {
Review Comment:
> Or are we wanting user to be able to specify a 'NULL' string
Yes, user can say, NULL and NULL means something different than the `YAML`
value `null|~|Null`
like (not the best example as this is not overridable, but it helps to see
how it behaves):
```yaml
ts:
dns:
resolv_conf: "NULL"
```
this part of the plugin seems to mimic what ats does
[internally](https://github.com/apache/trafficserver/blob/6cfb2c4d0a62b383a1375f3ba109e70d14137126/src/records/RecUtils.cc#L360)
for string record parsing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]