diff --git a/src/api/CMakeLists.txt b/src/api/CMakeLists.txt index ca3ea344862..fcfc12b67dc 100644 --- a/src/api/CMakeLists.txt +++ b/src/api/CMakeLists.txt @@ -63,4 +63,37 @@ if(APPLE) target_link_options(tsapi PRIVATE -undefined dynamic_lookup) endif() +if(BUILD_TESTING) + add_executable(test_api unit_tests/test_HttpOverridableConfig.cc) + target_link_libraries( + test_api + PRIVATE Catch2::Catch2WithMain + ts::tscore + ts::tsapi + ts::overridable_txn_vars + ts::tsutil + ts::http + ts::http_remap + ts::http2 + ts::logging + ts::hdrs + ts::diagsconfig + ts::inkutils + ts::inkdns + ts::inkhostdb + ts::inkcache + ts::aio + ts::proxy + ts::inknet + ts::records + ts::inkevent + libswoc::libswoc + ts::jsonrpc_protocol + ts::jsonrpc_server + ts::rpcpublichandlers + ts::configmanager + ) + add_catch2_test(NAME test_api COMMAND test_api) +endif() + clang_tidy_check(tsapi) diff --git a/src/api/InkAPITest.cc b/src/api/InkAPITest.cc index eac60ccd3fc..3739da52399 100644 --- a/src/api/InkAPITest.cc +++ b/src/api/InkAPITest.cc @@ -54,8 +54,6 @@ #include "records/RecHttp.h" #include "proxy/http/HttpSM.h" -#include "proxy/http/OverridableConfigDefs.h" -#include "iocore/net/ConnectionTracker.h" #include "tscore/TestBox.h" namespace @@ -94,7 +92,8 @@ DbgCtl dbg_ctl_SockClient{"SockClient"}; #define ERROR_BODY "TESTING ERROR PAGE" #define TRANSFORM_APPEND_STRING "This is a transformed response" -extern int dns_failover_period; +extern int dns_failover_period; +extern ClassAllocator httpSMAllocator; ////////////////////////////////////////////////////////////////////////////// // STRUCTURES @@ -8704,140 +8703,6 @@ EXCLUSIVE_REGRESSION_TEST(SDK_API_TSHttpConnectServerIntercept)(RegressionTest * return; } -//////////////////////////////////////////////// -// SDK_API_OVERRIDABLE_CONFIGS -// -// Unit Test for API: TSHttpTxnConfigFind -// TSHttpTxnConfigIntSet -// TSHttpTxnConfigIntGet -// TSHttpTxnConfigFloatSet -// TSHttpTxnConfigFloatGet -// TSHttpTxnConfigStringSet -// TSHttpTxnConfigStringGet -//////////////////////////////////////////////// - -// Generate the SDK_Overridable_Configs array from the X-macro. -// The order MUST match TSOverridableConfigKey enum order (enforced by static_assert). -// clang-format off -#define X_SDK_CONFIG(CONFIG_KEY, MEMBER, RECORD_NAME, DATA_TYPE, CONV) RECORD_NAME, -std::array SDK_Overridable_Configs = {{ - OVERRIDABLE_CONFIGS(X_SDK_CONFIG) -}}; -#undef X_SDK_CONFIG -// clang-format on - -static_assert(SDK_Overridable_Configs.size() == TS_CONFIG_LAST_ENTRY, - "SDK_Overridable_Configs size must match TS_CONFIG_LAST_ENTRY"); - -extern ClassAllocator httpSMAllocator; - -REGRESSION_TEST(SDK_API_OVERRIDABLE_CONFIGS)(RegressionTest *test, int /* atype ATS_UNUSED */, int *pstatus) -{ - TSOverridableConfigKey key; - TSRecordDataType type; - HttpSM *s = THREAD_ALLOC(httpSMAllocator, this_thread()); - bool success = true; - TSHttpTxn txnp = reinterpret_cast(s); - InkRand generator(17); - TSMgmtInt ival_read, ival_rand; - TSMgmtFloat fval_read, fval_rand; - const char *sval_read; - const char *test_string = "The Apache Traffic Server"; - int len; - - s->init(); - s->mutex = new_ProxyMutex(); - SCOPED_MUTEX_LOCK(lock, s->mutex, this_ethread()); - - HttpCacheSM *c_sm = &(s->get_cache_sm()); - c_sm->init(s, s->mutex); - - *pstatus = REGRESSION_TEST_INPROGRESS; - for (int i = 0; i < static_cast(SDK_Overridable_Configs.size()); ++i) { - std::string_view conf{SDK_Overridable_Configs[i]}; - - if (TS_SUCCESS == TSHttpTxnConfigFind(conf.data(), -1, &key, &type)) { - if (key != i) { - SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_FAIL, "Failed on %s, expected %d, got %d", conf.data(), i, key); - success = false; - continue; - } - } else { - SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_FAIL, "Call returned unexpected TS_ERROR for %s", conf.data()); - success = false; - continue; - } - - if (TS_SUCCESS == TSHttpTxnConfigFind(conf.data(), conf.size(), &key, &type)) { - if (key != i) { - SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_FAIL, "Failed on %s, expected %d, got %d", conf.data(), i, key); - success = false; - continue; - } - } else { - SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_FAIL, "Call returned unexpected TS_ERROR for %s", conf.data()); - success = false; - continue; - } - - // Now check the getters / setters - switch (type) { - case TS_RECORDDATATYPE_INT: - ival_rand = generator.random() % 126; // to fit in a signed byte - TSHttpTxnConfigIntSet(txnp, key, ival_rand); - TSHttpTxnConfigIntGet(txnp, key, &ival_read); - if (ival_rand != ival_read) { - SDK_RPRINT(test, "TSHttpTxnConfigIntSet", "TestCase1", TC_FAIL, "Failed on %s, %d != %d", conf.data(), ival_read, - ival_rand); - success = false; - continue; - } - break; - - case TS_RECORDDATATYPE_FLOAT: - fval_rand = generator.random(); - TSHttpTxnConfigFloatSet(txnp, key, fval_rand); - TSHttpTxnConfigFloatGet(txnp, key, &fval_read); - if (fval_rand != fval_read) { - SDK_RPRINT(test, "TSHttpTxnConfigFloatSet", "TestCase1", TC_FAIL, "Failed on %s, %f != %f", conf.data(), fval_read, - fval_rand); - success = false; - continue; - } - break; - - case TS_RECORDDATATYPE_STRING: - TSHttpTxnConfigStringSet(txnp, key, test_string, -1); - TSHttpTxnConfigStringGet(txnp, key, &sval_read, &len); - // Compare string content, not pointers - the implementation may store - // a copy of the string (e.g., in ParsedConfigCache for efficiency). - if (sval_read == nullptr || std::string_view(test_string) != std::string_view(sval_read, len)) { - SDK_RPRINT(test, "TSHttpTxnConfigStringSet", "TestCase1", TC_FAIL, "Failed on %s, %s != %s", conf.data(), - sval_read ? sval_read : "(null)", test_string); - success = false; - continue; - } - break; - - default: - break; - } - } - - s->destroy(); - if (success) { - *pstatus = REGRESSION_TEST_PASSED; - SDK_RPRINT(test, "TSHttpTxnConfigFind", "TestCase1", TC_PASS, "ok"); - SDK_RPRINT(test, "TSHttpTxnConfigIntSet", "TestCase1", TC_PASS, "ok"); - SDK_RPRINT(test, "TSHttpTxnConfigFloatSet", "TestCase1", TC_PASS, "ok"); - SDK_RPRINT(test, "TSHttpTxnConfigStringSet", "TestCase1", TC_PASS, "ok"); - } else { - *pstatus = REGRESSION_TEST_FAILED; - } - - return; -} - //////////////////////////////////////////////// // SDK_API_TXN_HTTP_INFO_INFO_GET // diff --git a/src/api/unit_tests/test_HttpOverridableConfig.cc b/src/api/unit_tests/test_HttpOverridableConfig.cc new file mode 100644 index 00000000000..4166ee38eee --- /dev/null +++ b/src/api/unit_tests/test_HttpOverridableConfig.cc @@ -0,0 +1,144 @@ +/** @file + + Catch based unit tests for HTTP overridable configuration APIs. + + @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. + */ + +#include "iocore/eventsystem/ConfigProcessor.h" +#include "iocore/net/ConnectionTracker.h" +#include "proxy/http/HttpConfig.h" +#include "proxy/http/HttpSM.h" +#include "proxy/http/OverridableConfigDefs.h" +#include "ts/ts.h" + +#include + +#include +#include + +using namespace std::literals; + +namespace +{ +struct ConfigDescriptor { + std::string_view name; + TSOverridableConfigKey key; + TSRecordDataType type; +}; + +// clang-format off +static constexpr std::array CONFIG_DESCRIPTORS{{ +#define X_CONFIG_DESCRIPTOR(CONFIG_KEY, MEMBER, RECORD_NAME, DATA_TYPE, CONV) \ + {RECORD_NAME, TS_CONFIG_##CONFIG_KEY, TS_RECORDDATATYPE_##DATA_TYPE}, + OVERRIDABLE_CONFIGS(X_CONFIG_DESCRIPTOR) +#undef X_CONFIG_DESCRIPTOR +}}; +// clang-format on + +class TestHttpTxn +{ +public: + TestHttpTxn() + { + if (HttpConfig::m_id == 0) { + HttpConfig::m_id = configProcessor.set(HttpConfig::m_id, new HttpConfigParams); + } + + _sm.magic = HttpSmMagic_t::ALIVE; + _sm.t_state.http_config_param = HttpConfig::acquire(); + REQUIRE(_sm.t_state.http_config_param != nullptr); + _sm.t_state.txn_conf = &_sm.t_state.http_config_param->oride; + } + + operator TSHttpTxn() { return reinterpret_cast(&_sm); } + +private: + HttpSM _sm; +}; +} // namespace + +TEST_CASE("Find HTTP overridable configurations", "[api][overridable-config]") +{ + for (auto const &descriptor : CONFIG_DESCRIPTORS) { + TSOverridableConfigKey key; + TSRecordDataType type; + + INFO(descriptor.name); + + REQUIRE(TSHttpTxnConfigFind(descriptor.name.data(), -1, &key, &type) == TS_SUCCESS); + CHECK(key == descriptor.key); + CHECK(type == descriptor.type); + + REQUIRE(TSHttpTxnConfigFind(descriptor.name.data(), descriptor.name.size(), &key, &type) == TS_SUCCESS); + CHECK(key == descriptor.key); + CHECK(type == descriptor.type); + } + + TSOverridableConfigKey key; + TSRecordDataType type; + + CHECK(TSHttpTxnConfigFind("proxy.config.invalid", -1, &key, &type) == TS_ERROR); +} + +TEST_CASE("Set and get HTTP overridable configurations", "[api][overridable-config]") +{ + TestHttpTxn txn; + + SECTION("integer") + { + static constexpr TSMgmtInt expected = 0; + TSMgmtInt actual; + + REQUIRE(TSHttpTxnConfigIntSet(txn, TS_CONFIG_HTTP_CACHE_HTTP, expected) == TS_SUCCESS); + REQUIRE(TSHttpTxnConfigIntGet(txn, TS_CONFIG_HTTP_CACHE_HTTP, &actual) == TS_SUCCESS); + CHECK(actual == expected); + } + + SECTION("float") + { + static constexpr TSMgmtFloat expected = 0.25; + TSMgmtFloat actual; + + REQUIRE(TSHttpTxnConfigFloatSet(txn, TS_CONFIG_HTTP_CACHE_HEURISTIC_LM_FACTOR, expected) == TS_SUCCESS); + REQUIRE(TSHttpTxnConfigFloatGet(txn, TS_CONFIG_HTTP_CACHE_HEURISTIC_LM_FACTOR, &actual) == TS_SUCCESS); + CHECK(actual == expected); + } + + SECTION("constrained integer") + { + TSMgmtInt actual; + + REQUIRE(TSHttpTxnConfigIntSet(txn, TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH, 95) == TS_SUCCESS); + REQUIRE(TSHttpTxnConfigIntGet(txn, TS_CONFIG_HTTP_PER_SERVER_CONNECTION_MATCH, &actual) == TS_SUCCESS); + CHECK(actual == TS_SERVER_OUTBOUND_MATCH_BOTH); + } + + SECTION("string") + { + static constexpr auto expected = "Catch test"sv; + const char *actual; + int length; + + REQUIRE(TSHttpTxnConfigStringSet(txn, TS_CONFIG_HTTP_RESPONSE_SERVER_STR, expected.data(), expected.size()) == TS_SUCCESS); + REQUIRE(TSHttpTxnConfigStringGet(txn, TS_CONFIG_HTTP_RESPONSE_SERVER_STR, &actual, &length) == TS_SUCCESS); + REQUIRE(actual != nullptr); + CHECK(std::string_view(actual, length) == expected); + } +} diff --git a/src/iocore/net/CMakeLists.txt b/src/iocore/net/CMakeLists.txt index b317e26b3ea..dba611fabb4 100644 --- a/src/iocore/net/CMakeLists.txt +++ b/src/iocore/net/CMakeLists.txt @@ -144,6 +144,7 @@ if(BUILD_TESTING) test_net libinknet_stub.cc NetVCTest.cc + unit_tests/test_ConnectionTracker.cc unit_tests/test_ProxyProtocol.cc unit_tests/test_SSLCertLookup.cc unit_tests/test_SSLNetVConnectionAsyncEp.cc diff --git a/src/iocore/net/ConnectionTracker.cc b/src/iocore/net/ConnectionTracker.cc index 45ce7e60f07..e6246e544b5 100644 --- a/src/iocore/net/ConnectionTracker.cc +++ b/src/iocore/net/ConnectionTracker.cc @@ -26,6 +26,8 @@ #include "records/RecCore.h" #include "swoc/IPAddr.h" +#include + using namespace std::literals; ConnectionTracker::TableSingleton ConnectionTracker::_inbound_table; @@ -49,11 +51,9 @@ const MgmtConverter ConnectionTracker::MIN_SERVER_CONV( const MgmtConverter ConnectionTracker::SERVER_MATCH_CONV{ [](const void *data) -> MgmtInt { return static_cast(*static_cast(data)); }, [](void *data, MgmtInt i) -> void { - // Problem - the InkAPITest requires being able to set an arbitrary value, so this can either - // correctly clamp or pass the regression tests. Currently it passes the tests. - // *static_cast(data) = std::clamp(static_cast(i), MATCH_IP, - // MATCH_BOTH); - *static_cast(data) = static_cast(i); + auto const value = std::clamp(i, static_cast(MATCH_IP), static_cast(MATCH_BOTH)); + + *static_cast(data) = static_cast(value); }, nullptr, nullptr, diff --git a/src/iocore/net/unit_tests/test_ConnectionTracker.cc b/src/iocore/net/unit_tests/test_ConnectionTracker.cc new file mode 100644 index 00000000000..ccd9d169d6f --- /dev/null +++ b/src/iocore/net/unit_tests/test_ConnectionTracker.cc @@ -0,0 +1,63 @@ +/** @file + + Catch based unit tests for connection tracking configuration. + + @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. + */ + +#include "iocore/net/ConnectionTracker.h" + +#include + +#include + +TEST_CASE("Connection tracker server match conversion", "[libinknet][ConnectionTracker]") +{ + auto const &converter = ConnectionTracker::SERVER_MATCH_CONV; + auto match = ConnectionTracker::MATCH_IP; + + REQUIRE(converter.store_int != nullptr); + REQUIRE(converter.load_int != nullptr); + + SECTION("valid values round trip") + { + static constexpr std::array valid_values{ + ConnectionTracker::MATCH_IP, + ConnectionTracker::MATCH_PORT, + ConnectionTracker::MATCH_HOST, + ConnectionTracker::MATCH_BOTH, + }; + + for (auto const expected : valid_values) { + converter.store_int(&match, static_cast(expected)); + + CHECK(match == expected); + CHECK(converter.load_int(&match) == static_cast(expected)); + } + } + + SECTION("invalid values are clamped") + { + converter.store_int(&match, -1); + CHECK(match == ConnectionTracker::MATCH_IP); + + converter.store_int(&match, 95); + CHECK(match == ConnectionTracker::MATCH_BOTH); + } +}