diff --git a/CODEOWNERS b/CODEOWNERS index 200d8fbf9e307..17fd742943a3d 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -77,6 +77,7 @@ extensions/filters/common/original_src @snowp @klarose # common crypto extension /*/extensions/common/crypto @lizan @PiotrSikora @bdecoste /*/extensions/common/proxy_protocol @alyssawilk @wez470 +/*/extensions/common/sqlutils @cpakulski @dio /*/extensions/filters/http/grpc_http1_bridge @snowp @jose /*/extensions/filters/http/gzip @gsagula @dio /*/extensions/filters/http/fault @rshriram @alyssawilk diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 8c8676f7b90b6..4127baaf1d87f 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -132,10 +132,10 @@ DEPENDENCY_REPOSITORIES = dict( cpe = "N/A", ), com_github_envoyproxy_sqlparser = dict( - sha256 = "b2d3882698cf85b64c87121e208ce0b24d5fe2a00a5d058cf4571f1b25b45403", - strip_prefix = "sql-parser-b14d010afd4313f2372a1cc96aa2327e674cc798", - # 2020-01-10 - urls = ["https://github.com/envoyproxy/sql-parser/archive/b14d010afd4313f2372a1cc96aa2327e674cc798.tar.gz"], + sha256 = "96c10c8e950a141a32034f19b19cdeb1da48fe859cf96ae5e19f894f36c62c71", + strip_prefix = "sql-parser-3b40ba2d106587bdf053a292f7e3bb17e818a57f", + # 2020-06-10 + urls = ["https://github.com/envoyproxy/sql-parser/archive/3b40ba2d106587bdf053a292f7e3bb17e818a57f.tar.gz"], use_category = ["dataplane"], cpe = "N/A", ), diff --git a/source/extensions/common/sqlutils/BUILD b/source/extensions/common/sqlutils/BUILD new file mode 100644 index 0000000000000..c0129c29cfc36 --- /dev/null +++ b/source/extensions/common/sqlutils/BUILD @@ -0,0 +1,19 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_library( + name = "sqlutils_lib", + srcs = ["sqlutils.cc"], + hdrs = ["sqlutils.h"], + external_deps = ["sqlparser"], + deps = [ + "//source/common/protobuf:utility_lib", + ], +) diff --git a/source/extensions/common/sqlutils/sqlutils.cc b/source/extensions/common/sqlutils/sqlutils.cc new file mode 100644 index 0000000000000..023a5529e0830 --- /dev/null +++ b/source/extensions/common/sqlutils/sqlutils.cc @@ -0,0 +1,40 @@ +#include "extensions/common/sqlutils/sqlutils.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace SQLUtils { + +bool SQLUtils::setMetadata(const std::string& query, ProtobufWkt::Struct& metadata) { + hsql::SQLParserResult result; + hsql::SQLParser::parse(query, &result); + + if (!result.isValid()) { + return false; + } + + auto& fields = *metadata.mutable_fields(); + + for (auto i = 0u; i < result.size(); ++i) { + if (result.getStatement(i)->type() == hsql::StatementType::kStmtShow) { + continue; + } + hsql::TableAccessMap table_access_map; + // Get names of accessed tables. + result.getStatement(i)->tablesAccessed(table_access_map); + for (auto& it : table_access_map) { + auto& operations = *fields[it.first].mutable_list_value(); + // For each table get names of operations performed on that table. + for (const auto& ot : it.second) { + operations.add_values()->set_string_value(ot); + } + } + } + + return true; +} + +} // namespace SQLUtils +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/common/sqlutils/sqlutils.h b/source/extensions/common/sqlutils/sqlutils.h new file mode 100644 index 0000000000000..8519f21836fa7 --- /dev/null +++ b/source/extensions/common/sqlutils/sqlutils.h @@ -0,0 +1,26 @@ +#include "common/protobuf/utility.h" + +#include "include/sqlparser/SQLParser.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace SQLUtils { + +class SQLUtils { +public: + /** + * Method parses SQL query string and writes output to metadata. + * @param query supplies SQL statement. + * @param metadata supplies placeholder where metadata should be written. + * @return True if parsing was successful and False if parsing failed. + * If True was returned the metadata contains result of parsing. The results are + * stored in metadata.mutable_fields. + **/ + static bool setMetadata(const std::string& query, ProtobufWkt::Struct& metadata); +}; + +} // namespace SQLUtils +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/network/mysql_proxy/BUILD b/source/extensions/filters/network/mysql_proxy/BUILD index 99b5ebdd8ae97..152584385054a 100644 --- a/source/extensions/filters/network/mysql_proxy/BUILD +++ b/source/extensions/filters/network/mysql_proxy/BUILD @@ -36,7 +36,6 @@ envoy_cc_library( "mysql_session.h", "mysql_utils.h", ], - external_deps = ["sqlparser"], deps = [ "//include/envoy/network:filter_interface", "//include/envoy/server:filter_config_interface", @@ -44,6 +43,7 @@ envoy_cc_library( "//include/envoy/stats:stats_macros", "//source/common/buffer:buffer_lib", "//source/common/network:filter_lib", + "//source/extensions/common/sqlutils:sqlutils_lib", "//source/extensions/filters/network:well_known_names", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], diff --git a/source/extensions/filters/network/mysql_proxy/mysql_filter.cc b/source/extensions/filters/network/mysql_proxy/mysql_filter.cc index 0d8be2394d150..648171e786e31 100644 --- a/source/extensions/filters/network/mysql_proxy/mysql_filter.cc +++ b/source/extensions/filters/network/mysql_proxy/mysql_filter.cc @@ -6,10 +6,9 @@ #include "common/common/assert.h" #include "common/common/logger.h" +#include "extensions/common/sqlutils/sqlutils.h" #include "extensions/filters/network/well_known_names.h" -#include "include/sqlparser/SQLParser.h" - namespace Envoy { namespace Extensions { namespace NetworkFilters { @@ -105,39 +104,21 @@ void MySQLFilter::onCommand(Command& command) { } // Parse a given query - hsql::SQLParserResult result; - hsql::SQLParser::parse(command.getData(), &result); + envoy::config::core::v3::Metadata& dynamic_metadata = + read_callbacks_->connection().streamInfo().dynamicMetadata(); + ProtobufWkt::Struct metadata( + (*dynamic_metadata.mutable_filter_metadata())[NetworkFilterNames::get().MySQLProxy]); + auto result = Common::SQLUtils::SQLUtils::setMetadata(command.getData(), metadata); ENVOY_CONN_LOG(trace, "mysql_proxy: query processed {}", read_callbacks_->connection(), command.getData()); - if (!result.isValid()) { + if (!result) { config_->stats_.queries_parse_error_.inc(); return; } config_->stats_.queries_parsed_.inc(); - // Set dynamic metadata - envoy::config::core::v3::Metadata& dynamic_metadata = - read_callbacks_->connection().streamInfo().dynamicMetadata(); - ProtobufWkt::Struct metadata( - (*dynamic_metadata.mutable_filter_metadata())[NetworkFilterNames::get().MySQLProxy]); - auto& fields = *metadata.mutable_fields(); - - for (auto i = 0u; i < result.size(); ++i) { - if (result.getStatement(i)->type() == hsql::StatementType::kStmtShow) { - continue; - } - hsql::TableAccessMap table_access_map; - result.getStatement(i)->tablesAccessed(table_access_map); - for (auto& it : table_access_map) { - auto& operations = *fields[it.first].mutable_list_value(); - for (const auto& ot : it.second) { - operations.add_values()->set_string_value(ot); - } - } - } - read_callbacks_->connection().streamInfo().setDynamicMetadata( NetworkFilterNames::get().MySQLProxy, metadata); } diff --git a/test/extensions/common/sqlutils/BUILD b/test/extensions/common/sqlutils/BUILD new file mode 100644 index 0000000000000..0277e47706b2d --- /dev/null +++ b/test/extensions/common/sqlutils/BUILD @@ -0,0 +1,20 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_package() + +envoy_cc_test( + name = "sqlutils_tests", + srcs = [ + "sqlutils_test.cc", + ], + external_deps = ["sqlparser"], + deps = [ + "//source/extensions/common/sqlutils:sqlutils_lib", + ], +) diff --git a/test/extensions/common/sqlutils/sqlutils_test.cc b/test/extensions/common/sqlutils/sqlutils_test.cc new file mode 100644 index 0000000000000..74ce5c8dfef81 --- /dev/null +++ b/test/extensions/common/sqlutils/sqlutils_test.cc @@ -0,0 +1,182 @@ +#include "extensions/common/sqlutils/sqlutils.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace SQLUtils { + +// MetadataFromSQLTest class is used for parameterized tests. +// The values in the tests are: +// std::string - SQL query +// bool - whether to expect SQL parsing to be successful +// std::map> map of expected tables accessed based on the query. +// The map is checked only when parsing was successful. Map is indexed by table name and points to +// list of operations performed on the table. For example table1: "select", "insert" says that there +// was SELECT and INSERT operations on table1. +class MetadataFromSQLTest + : public ::testing::TestWithParam< + std::tuple>>> {}; + +// Test takes SQL query as a parameter and checks if the parsing +// produces the correct metadata. +// Metadata is 2-level structure. First layer is list of resources +// over which the SQL query operates: in our case is list of tables. +// Under each table there is secondary list which contains operations performed +// on the table, like "select", "insert", etc. +TEST_P(MetadataFromSQLTest, ParsingAndMetadataTest) { + // Get the SQL query + const std::string& query = std::get<0>(GetParam()); + // vector of queries to check. + std::vector test_queries; + test_queries.push_back(query); + + // Create uppercase and lowercase versions of the queries and put + // them into vector of queries to check + test_queries.push_back(absl::AsciiStrToLower(query)); + test_queries.push_back(absl::AsciiStrToUpper(query)); + + while (!test_queries.empty()) { + std::string test_query = test_queries.back(); + ProtobufWkt::Struct metadata; + + // Check if the parsing result is what expected. + ASSERT_EQ(std::get<1>(GetParam()), SQLUtils::setMetadata(test_query, metadata)); + + // If parsing was expected to fail do not check parsing values. + if (!std::get<1>(GetParam())) { + return; + } + + // Access metadata fields, where parsing results are stored. + auto& fields = *metadata.mutable_fields(); + + // Get the names of resources which SQL query operates on. + std::map> expected_tables = std::get<2>(GetParam()); + // Check if query results return the same number of resources as expected. + ASSERT_EQ(expected_tables.size(), fields.size()); + for (const auto& i : fields) { + // Get from created metadata the list of operations on the resource + const auto& operations = i; + std::string table_name = operations.first; + + std::transform(table_name.begin(), table_name.end(), table_name.begin(), + [](unsigned char c) { return std::tolower(c); }); + // Get the list of expected operations on the same resource from test param. + const auto& table_name_it = expected_tables.find(table_name); + // Make sure that a resource (table) found in metadata is expected. + ASSERT_NE(expected_tables.end(), table_name_it); + auto& operations_list = table_name_it->second; + // The number of expected operations and created in metadata must be the same. + ASSERT_EQ(operations_list.size(), operations.second.list_value().values().size()); + // Now iterate over the operations list found in metadata and check if the same operation + // is listed as expected in test param. + for (const auto& j : operations.second.list_value().values()) { + // Find that operation in test params. + const auto operation_it = + std::find(operations_list.begin(), operations_list.end(), j.string_value()); + ASSERT_NE(operations_list.end(), operation_it); + // Erase the operation. At the end of the test this list should be empty what means + // that we found all expected operations. + operations_list.erase(operation_it); + } + // Make sure that we went through all expected operations. + ASSERT_TRUE(operations_list.empty()); + // Remove the table from the list. At the end of the test this list must be empty. + expected_tables.erase(table_name_it); + } + + ASSERT_TRUE(expected_tables.empty()); + test_queries.pop_back(); + } +} + +// Note: This parameterized test's queries are converted to all lowercase and all uppercase +// to validate that parser is case-insensitive. The test routine converts to uppercase and +// lowercase entire query string, not only SQL keywords. This introduces a problem when comparing +// tables' names when verifying parsing result. Therefore the test converts table names to lowercase +// before comparing. It however requires that all table names in the queries below use lowercase +// only. +#define TEST_VALUE(...) \ + std::tuple>> { __VA_ARGS__ } +INSTANTIATE_TEST_SUITE_P( + SQLUtilsTestSuite, MetadataFromSQLTest, + ::testing::Values( + TEST_VALUE("blahblah;", false, {}), + + TEST_VALUE("CREATE TABLE IF NOT EXISTS table1(Usr VARCHAR(40),Count INT);", true, + {{"table1", {"create"}}}), + TEST_VALUE("CREATE TABLE IF NOT EXISTS `table number 1`(Usr VARCHAR(40),Count INT);", true, + {{"table number 1", {"create"}}}), + TEST_VALUE( + "CREATE TABLE IF NOT EXISTS table1(Usr VARCHAR(40),Count INT); SELECT * from table1;", + true, {{"table1", {"select", "create"}}}), + TEST_VALUE( + "CREATE TABLE IF NOT EXISTS table1(Usr VARCHAR(40),Count INT); SELECT * from table2;", + true, {{"table1", {"create"}}, {"table2", {"select"}}}), + + TEST_VALUE("CREATE TABLE table1(Usr VARCHAR(40),Count INT);", true, + {{"table1", {"create"}}}), + TEST_VALUE("CREATE TABLE;", false, {}), + TEST_VALUE("CREATE TEMPORARY table table1(Usr VARCHAR(40),Count INT);", true, + {{"table1", {"create"}}}), + TEST_VALUE("DROP TABLE IF EXISTS table1", true, {{"table1", {"drop"}}}), + TEST_VALUE("ALTER TABLE table1 add column Id varchar (20);", true, {{"table1", {"alter"}}}), + TEST_VALUE("INSERT INTO table1 (Usr, Count) VALUES ('allsp2', 3);", true, + {{"table1", {"insert"}}}), + TEST_VALUE("INSERT LOW_PRIORITY INTO table1 (Usr, Count) VALUES ('allsp2', 3);", true, + {{"table1", {"insert"}}}), + TEST_VALUE("INSERT IGNORE INTO table1 (Usr, Count) VALUES ('allsp2', 3);", true, + {{"table1", {"insert"}}}), + TEST_VALUE("INSERT INTO table1 (Usr, Count) VALUES ('allsp2', 3);SELECT * from table1", + true, {{"table1", {"insert", "select"}}}), + TEST_VALUE("DELETE FROM table1 WHERE Count > 3;", true, {{"table1", {"delete"}}}), + TEST_VALUE("DELETE LOW_PRIORITY FROM table1 WHERE Count > 3;", true, + {{"table1", {"delete"}}}), + TEST_VALUE("DELETE QUICK FROM table1 WHERE Count > 3;", true, {{"table1", {"delete"}}}), + TEST_VALUE("DELETE IGNORE FROM table1 WHERE Count > 3;", true, {{"table1", {"delete"}}}), + + TEST_VALUE("SELECT * FROM table1 WHERE Count = 1;", true, {{"table1", {"select"}}}), + TEST_VALUE("SELECT * FROM table1 WHERE Count = 1;", true, {{"table1", {"select"}}}), + TEST_VALUE("SELECT product.category FROM table1 WHERE Count = 1;", true, + {{"table1", {"select"}}, {"product", {"unknown"}}}), + TEST_VALUE("SELECT DISTINCT Usr FROM table1;", true, {{"table1", {"select"}}}), + TEST_VALUE("SELECT Usr, Count FROM table1 ORDER BY Count DESC;", true, + {{"table1", {"select"}}}), + TEST_VALUE("SELECT 12 AS a, a FROM table1 GROUP BY a;", true, {{"table1", {"select"}}}), + TEST_VALUE("SELECT;", false, {}), TEST_VALUE("SELECT Usr, Count FROM;", false, {}), + TEST_VALUE("INSERT INTO table1 SELECT * FROM table2;", true, + {{"table1", {"insert"}}, {"table2", {"select"}}}), + TEST_VALUE("INSERT INTO table1 SELECT tbl_temp1.fld_order_id FROM table2;", true, + {{"tbl_temp1", {"unknown"}}, {"table2", {"select"}}, {"table1", {"insert"}}}), + TEST_VALUE("UPDATE table1 SET col1 = col1 + 1", true, {{"table1", {"update"}}}), + TEST_VALUE("UPDATE LOW_PRIORITY table1 SET col1 = col1 + 1", true, + {{"table1", {"update"}}}), + TEST_VALUE("UPDATE IGNORE table1 SET col1 = col1 + 1", true, {{"table1", {"update"}}}), + TEST_VALUE("UPDATE table1 SET column1=(SELECT * columnX from table2);", true, + {{"table1", {"update"}}, {"table2", {"select"}}}), + + // operations on database should not create any metadata + TEST_VALUE("CREATE DATABASE testdb;", true, {}), + TEST_VALUE("CREATE DATABASE IF NOT EXISTS testdb;", true, {}), + TEST_VALUE("ALTER DATABASE testdb CHARACTER SET charset_name;", true, {}), + TEST_VALUE("ALTER DATABASE testdb default CHARACTER SET charset_name;", true, {}), + TEST_VALUE("ALTER DATABASE testdb default CHARACTER SET = charset_name;", true, {}), + TEST_VALUE("ALTER SCHEMA testdb default CHARACTER SET = charset_name;", true, {}), + + // The following DROP DATABASE tests should not produce metadata. + TEST_VALUE("DROP DATABASE testdb;", true, {}), + TEST_VALUE("DROP DATABASE IF EXISTS testdb;", true, {}), + + // Schema. Should be parsed fine, but should not produce any metadata + TEST_VALUE("SHOW databases;", true, {}), TEST_VALUE("SHOW tables;", true, {}), + TEST_VALUE("SELECT * FROM;", false, {}), + TEST_VALUE("SELECT 1 FROM tabletest1;", true, {{"tabletest1", {"select"}}}) + + )); + +} // namespace SQLUtils +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/network/mysql_proxy/mysql_command_test.cc b/test/extensions/filters/network/mysql_proxy/mysql_command_test.cc index cce430facd773..55cacdc2ab532 100644 --- a/test/extensions/filters/network/mysql_proxy/mysql_command_test.cc +++ b/test/extensions/filters/network/mysql_proxy/mysql_command_test.cc @@ -203,7 +203,7 @@ class MySQLCommandTest : public testing::Test, public MySQLTestUtils { EXPECT_EQ(1UL, result.size()); EXPECT_EQ(statement_type, result.getStatement(0)->type()); hsql::TableAccessMap table_access_map; - if (expected_table_access_map.empty()) { + if (expected_table_access_map.empty() && (statement_type == hsql::StatementType::kStmtShow)) { return; } result.getStatement(0)->tablesAccessed(table_access_map); @@ -454,7 +454,8 @@ TEST_F(MySQLCommandTest, MySQLTest20) { std::string command = buildAlter(TestResource::TABLE, table, "add column Id varchar (20)"); hsql::SQLParserResult result; EXPECT_EQ(MYSQL_SUCCESS, encodeQuery(command, result)); - expectStatementTypeAndTableAccessMap(result, hsql::StatementType::kStmtAlter, {}); + expectStatementTypeAndTableAccessMap(result, hsql::StatementType::kStmtAlter, + {{table, {"alter"}}}); } /*