Break out ConfigResult

Author: bneradt

include/config/config_result.h

@@ -0,0 +1,77 @@
+/** @file
+
+  Generic configuration parsing result type.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+#pragma once
+
+#include "swoc/Errata.h"
+
+namespace config
+{
+
+/**
+ * Result of a configuration parse operation.
+ *
+ * This template is the standard return type for all configuration parsers in
+ * the config library. It bundles a parsed configuration value with an errata
+ * that may contain warnings or errors encountered during parsing.
+ *
+ * Design rationale:
+ * - Parsers can return partial results (value populated) even when warnings
+ *   are present, allowing callers to decide how to handle degraded configs.
+ * - The ok() method checks whether parsing succeeded without errors, but
+ *   callers should also inspect the errata for warnings.
+ * - This type is reused across all configuration file types (ssl_multicert,
+ *   etc.) to provide a consistent API.
+ *
+ * Example usage:
+ * @code
+ *   config::SSLMultiCertParser parser;
+ *   auto result = parser.parse("/path/to/ssl_multicert.yaml");
+ *   if (!result.ok()) {
+ *       // Handle error
+ *       return;
+ *   }
+ *   for (const auto& entry : result.value) {
+ *       // Use parsed entries
+ *   }
+ * @endcode
+ *
+ * @tparam T The configuration type (e.g., SSLMultiCertConfig).
+ */
+template <typename T> struct ConfigResult {
+  T            value;  ///< The parsed configuration value.
+  swoc::Errata errata; ///< Errors or warnings from parsing.
+
+  /**
+   * Check if parsing succeeded without errors.
+   *
+   * @return true if no errors occurred, false otherwise.
+   */
+  bool
+  ok() const
+  {
+    return errata.is_ok();
+  }
+};
+
+} // namespace config

include/config/ssl_multicert.h

@@ -28,7 +28,7 @@
 #include <string_view>
 #include <vector>
 
-#include "swoc/Errata.h"
+#include "config/config_result.h"
 
 namespace config
 {
@@ -52,27 +52,6 @@ struct SSLMultiCertEntry {
 /// A configuration is a vector of certificate entries.
 using SSLMultiCertConfig = std::vector<SSLMultiCertEntry>;
 
-/**
- * Result of a configuration parse operation.
- *
- * @tparam T The configuration type.
- */
-template <typename T> struct ConfigResult {
-  T            value;  ///< The parsed configuration value.
-  swoc::Errata errata; ///< Errors or warnings from parsing.
-
-  /**
-   * Check if parsing succeeded without errors.
-   *
-   * @return true if no errors occurred, false otherwise.
-   */
-  bool
-  ok() const
-  {
-    return errata.is_ok();
-  }
-};
-
 /**
  * Parser for ssl_multicert configuration files.
  *

src/config/CMakeLists.txt

@@ -15,7 +15,9 @@
 #
 #######################
 
-set(CONFIG_PUBLIC_HEADERS ${PROJECT_SOURCE_DIR}/include/config/ssl_multicert.h)
+set(CONFIG_PUBLIC_HEADERS ${PROJECT_SOURCE_DIR}/include/config/config_result.h
+                          ${PROJECT_SOURCE_DIR}/include/config/ssl_multicert.h
+)
 
 add_library(tsconfig ssl_multicert.cc)
 

src/config/README.md

@@ -5,8 +5,7 @@ used by various ATS components including `traffic_server` and `traffic_ctl`.
 
 ## Architecture Overview
 
-Each configuration file type (ssl_multicert, logging, sni, etc.) follows the
-same pattern:
+Each configuration file type (ssl_multicert, etc.) follows the same pattern:
 
 ```
 include/config/<config_name>.h   - Header with data types, parser, marshaller
@@ -73,7 +72,7 @@ The parser automatically detects the configuration format:
 
 ## Adding a New Configuration Type
 
-To add support for a new configuration (e.g., `logging.yaml`):
+To add support for a new configuration (maybe, `logging.yaml`):
 
 1. Create `include/config/logging.h`:
    - Define `LoggingEntry` struct with fields matching the config

src/config/ssl_multicert.cc

@@ -27,7 +27,6 @@
 #include <cctype>
 #include <exception>
 #include <set>
-#include <sstream>
 
 #include <yaml-cpp/yaml.h>
 
@@ -100,43 +99,36 @@ parse_legacy_line(swoc::TextView line)
   return result;
 }
 
-/// Escape a string for YAML output if needed.
-std::string
-yaml_escape(std::string const &value)
+/// Emit a single SSLMultiCertEntry to a YAML::Emitter.
+void
+emit_entry(YAML::Emitter &emitter, config::SSLMultiCertEntry const &entry)
 {
-  if (value.empty()) {
-    return "\"\"";
-  }
+  emitter << YAML::BeginMap;
 
-  bool needs_quotes = false;
-  if (value[0] == '*' || value[0] == '!' || value[0] == '&' || value[0] == '{' || value[0] == '}' || value[0] == '[' ||
-      value[0] == ']' || value[0] == ',' || value[0] == '#' || value[0] == '?' || value[0] == '-' || value[0] == ':' ||
-      value[0] == '>' || value[0] == '|' || value[0] == '@' || value[0] == '`' || value[0] == '"' || value[0] == '\'') {
-    needs_quotes = true;
-  } else if (value.find(':') != std::string::npos || value.find('#') != std::string::npos) {
-    needs_quotes = true;
-  } else if (value == "true" || value == "false" || value == "yes" || value == "no" || value == "null" || value == "True" ||
-             value == "False" || value == "Yes" || value == "No" || value == "Null") {
-    needs_quotes = true;
-  }
+  auto write_field = [&](char const *key, std::string const &value) {
+    if (!value.empty()) {
+      emitter << YAML::Key << key << YAML::Value << value;
+    }
+  };
 
-  if (needs_quotes) {
-    std::string escaped;
-    escaped.reserve(value.size() + 2);
-    escaped += '"';
-    for (char c : value) {
-      if (c == '\\') {
-        escaped += "\\\\";
-      } else if (c == '"') {
-        escaped += "\\\"";
-      } else {
-        escaped += c;
-      }
+  auto write_int_field = [&](char const *key, std::optional<int> const &value) {
+    if (value.has_value()) {
+      emitter << YAML::Key << key << YAML::Value << value.value();
     }
-    escaped += '"';
-    return escaped;
-  }
-  return value;
+  };
+
+  write_field(KEY_SSL_CERT_NAME, entry.ssl_cert_name);
+  write_field(KEY_DEST_IP, entry.dest_ip);
+  write_field(KEY_SSL_KEY_NAME, entry.ssl_key_name);
+  write_field(KEY_SSL_CA_NAME, entry.ssl_ca_name);
+  write_field(KEY_SSL_OCSP_NAME, entry.ssl_ocsp_name);
+  write_field(KEY_SSL_KEY_DIALOG, entry.ssl_key_dialog);
+  write_field(KEY_DEST_FQDN, entry.dest_fqdn);
+  write_field(KEY_ACTION, entry.action);
+  write_int_field(KEY_SSL_TICKET_ENABLED, entry.ssl_ticket_enabled);
+  write_int_field(KEY_SSL_TICKET_NUMBER, entry.ssl_ticket_number);
+
+  emitter << YAML::EndMap;
 }
 
 } // namespace
@@ -268,9 +260,8 @@ SSLMultiCertParser::parse_yaml(std::string_view content)
       return {result, swoc::Errata("expected 'ssl_multicert' to be a sequence")};
     }
 
-    for (auto it = entries.begin(); it != entries.end(); ++it) {
-      YAML::Node entry_node = *it;
-      auto const mark       = entry_node.Mark();
+    for (auto const &entry_node : entries) {
+      auto const mark = entry_node.Mark();
       if (!entry_node.IsMap()) {
         return {result, swoc::Errata(ERRATA_ERROR_SEV, "Expected ssl_multicert entries to be maps at line {}, column {}", mark.line,
                                      mark.column)};
@@ -352,41 +343,16 @@ SSLMultiCertParser::parse_legacy(std::string_view content)
 std::string
 SSLMultiCertMarshaller::to_yaml(SSLMultiCertConfig const &config)
 {
-  std::ostringstream out;
-  out << "ssl_multicert:\n";
+  YAML::Emitter yaml;
+  yaml << YAML::BeginMap;
+  yaml << YAML::Key << KEY_SSL_MULTICERT << YAML::Value << YAML::BeginSeq;
 
   for (auto const &entry : config) {
-    bool first = true;
-
-    auto write_field = [&](char const *key, std::string const &value) {
-      if (value.empty()) {
-        return;
-      }
-      out << (first ? "  - " : "    ") << key << ": " << yaml_escape(value) << "\n";
-      first = false;
-    };
-
-    auto write_int_field = [&](char const *key, std::optional<int> const &value) {
-      if (!value.has_value()) {
-        return;
-      }
-      out << (first ? "  - " : "    ") << key << ": " << value.value() << "\n";
-      first = false;
-    };
-
-    write_field(KEY_SSL_CERT_NAME, entry.ssl_cert_name);
-    write_field(KEY_DEST_IP, entry.dest_ip);
-    write_field(KEY_SSL_KEY_NAME, entry.ssl_key_name);
-    write_field(KEY_SSL_CA_NAME, entry.ssl_ca_name);
-    write_field(KEY_SSL_OCSP_NAME, entry.ssl_ocsp_name);
-    write_field(KEY_SSL_KEY_DIALOG, entry.ssl_key_dialog);
-    write_field(KEY_DEST_FQDN, entry.dest_fqdn);
-    write_field(KEY_ACTION, entry.action);
-    write_int_field(KEY_SSL_TICKET_ENABLED, entry.ssl_ticket_enabled);
-    write_int_field(KEY_SSL_TICKET_NUMBER, entry.ssl_ticket_number);
+    emit_entry(yaml, entry);
   }
 
-  return out.str();
+  yaml << YAML::EndSeq << YAML::EndMap;
+  return yaml.c_str();
 }
 
 std::string
@@ -398,32 +364,7 @@ SSLMultiCertMarshaller::to_json(SSLMultiCertConfig const &config)
   json << YAML::Key << KEY_SSL_MULTICERT << YAML::Value << YAML::BeginSeq;
 
   for (auto const &entry : config) {
-    json << YAML::BeginMap;
-
-    auto write_field = [&](char const *key, std::string const &value) {
-      if (!value.empty()) {
-        json << YAML::Key << key << YAML::Value << value;
-      }
-    };
-
-    auto write_int_field = [&](char const *key, std::optional<int> const &value) {
-      if (value.has_value()) {
-        json << YAML::Key << key << YAML::Value << value.value();
-      }
-    };
-
-    write_field(KEY_SSL_CERT_NAME, entry.ssl_cert_name);
-    write_field(KEY_DEST_IP, entry.dest_ip);
-    write_field(KEY_SSL_KEY_NAME, entry.ssl_key_name);
-    write_field(KEY_SSL_CA_NAME, entry.ssl_ca_name);
-    write_field(KEY_SSL_OCSP_NAME, entry.ssl_ocsp_name);
-    write_field(KEY_SSL_KEY_DIALOG, entry.ssl_key_dialog);
-    write_field(KEY_DEST_FQDN, entry.dest_fqdn);
-    write_field(KEY_ACTION, entry.action);
-    write_int_field(KEY_SSL_TICKET_ENABLED, entry.ssl_ticket_enabled);
-    write_int_field(KEY_SSL_TICKET_NUMBER, entry.ssl_ticket_number);
-
-    json << YAML::EndMap;
+    emit_entry(json, entry);
   }
 
   json << YAML::EndSeq << YAML::EndMap;

src/config/unit_tests/test_ssl_multicert.cc

@@ -297,7 +297,7 @@ TEST_CASE("SSLMultiCertMarshaller produces valid JSON", "[ssl_multicert][marshal
   CHECK(json.find(']') != std::string::npos);
 }
 
-TEST_CASE("SSLMultiCertMarshaller escapes special characters", "[ssl_multicert][marshaller][escaping]")
+TEST_CASE("SSLMultiCertMarshaller handles special characters", "[ssl_multicert][marshaller][escaping]")
 {
   SSLMultiCertConfig config;
 
@@ -309,11 +309,16 @@ TEST_CASE("SSLMultiCertMarshaller escapes special characters", "[ssl_multicert][
 
   SSLMultiCertMarshaller marshaller;
 
-  SECTION("YAML escapes quotes")
+  SECTION("YAML output contains the field and can be re-parsed")
   {
     std::string yaml = marshaller.to_yaml(config);
     CHECK(yaml.find("ssl_key_dialog:") != std::string::npos);
-    CHECK(yaml.find("\\\"with quotes\\\"") != std::string::npos);
+
+    // Verify round-trip preserves the value.
+    auto result = parse_content(yaml, "test.yaml");
+    REQUIRE(result.ok());
+    REQUIRE(result.value.size() == 1);
+    CHECK(result.value[0].ssl_key_dialog == "exec:/path/to/script \"with quotes\"");
   }
 
   SECTION("JSON escapes quotes")

src/traffic_ctl/ConvertConfigCommand.cc

@@ -66,14 +66,14 @@ ConvertConfigCommand::convert_ssl_multicert()
 
   // Write to output file or stdout if output is "-".
   if (_output_file == "-") {
-    std::cout << yaml_output;
+    std::cout << yaml_output << '\n';
   } else {
     std::ofstream out(_output_file);
     if (!out) {
       _printer->write_output("Failed to open output file '" + _output_file + "' for writing");
       return;
     }
-    out << yaml_output;
+    out << yaml_output << '\n';
     out.close();
     _printer->write_output("Converted " + _input_file + " -> " + _output_file);
   }

tests/gold_tests/traffic_ctl/convert_ssl_multicert/gold/full.yaml

@@ -5,13 +5,13 @@ ssl_multicert:
     ssl_ticket_enabled: 1
     ssl_ticket_number: 3
   - ssl_cert_name: ecdsa.pem,rsa.pem
-    dest_ip: "10.0.0.1:443"
+    dest_ip: 10.0.0.1:443
     ssl_key_name: ecdsa.key,rsa.key
     ssl_ca_name: ca.pem
     ssl_ocsp_name: ocsp.der
   - ssl_cert_name: encrypted.pem
     dest_ip: "*"
-    ssl_key_dialog: "exec:/usr/bin/getpass foo"
+    ssl_key_dialog: exec:/usr/bin/getpass foo
   - dest_ip: 192.168.1.1
     action: tunnel
   - ssl_cert_name: wildcard.pem

tests/gold_tests/traffic_ctl/convert_ssl_multicert/gold/quoted.yaml

@@ -1,6 +1,6 @@
 ssl_multicert:
   - ssl_cert_name: server.pem
     dest_ip: "*"
-    ssl_key_dialog: "exec:/usr/bin/getpass arg1 'arg 2'"
+    ssl_key_dialog: exec:/usr/bin/getpass arg1 'arg 2'
   - ssl_cert_name: another.pem
     dest_ip: "[::1]:8443"

tests/gold_tests/traffic_ctl/show_ssl_multicert/gold/show_yaml.gold

@@ -2,4 +2,3 @@ ssl_multicert:
   - ssl_cert_name: server.pem
     dest_ip: "*"
     ssl_key_name: server.key
-