Improve KFS request validation

common_settings.bzl

@@ -252,6 +252,26 @@ COMMON_FUZZER_LINKOPTS = [
 ]
 COMMON_LOCAL_DEFINES = ["SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_TRACE"]
 COMMON_DEFINES = DEFINES_PYTHON + DEFINES_MEDIAPIPE
+COPTS_CLOUD = select({
+    "//conditions:default": ["-DCLOUD_DISABLE=1"],
+    "//:not_disable_cloud": ["-DCLOUD_DISABLE=0"],
+})
+COPTS_TESTS = COMMON_STATIC_TEST_COPTS + COPTS_CLOUD
+
+def ovms_cc_test_library(**kwargs):
+    """cc_library wrapper with defaults suited for test libraries (linkstatic, alwayslink, test copts)."""
+    if "copts" not in kwargs:
+        kwargs["copts"] = COPTS_TESTS
+    if "additional_copts" in kwargs:
+        kwargs["copts"] = kwargs["copts"] + kwargs.pop("additional_copts")
+    if "local_defines" not in kwargs:
+        kwargs["local_defines"] = COMMON_LOCAL_DEFINES
+    if "linkstatic" not in kwargs:
+        kwargs["linkstatic"] = 1
+    if "alwayslink" not in kwargs:
+        kwargs["alwayslink"] = True
+    native.cc_library(**kwargs)
+
 PYBIND_DEPS = [
     "//third_party:python3",
     "@pybind11//:pybind11_embed",

src/BUILD

@@ -17,7 +17,8 @@ load("@bazel_skylib//lib:selects.bzl", "selects")
 load("@mediapipe//mediapipe/framework:more_selects.bzl", "more_selects")
 load("//:common_settings.bzl",
      "COMMON_STATIC_TEST_COPTS", "COMMON_STATIC_LIBS_COPTS", "COMMON_STATIC_LIBS_LINKOPTS", "COMMON_FUZZER_COPTS", "COMMON_FUZZER_LINKOPTS", "COMMON_LOCAL_DEFINES",
-     "create_config_settings", "PYBIND_DEPS", "ovms_cc_library")
+     "COPTS_CLOUD", "COPTS_TESTS",
+     "create_config_settings", "PYBIND_DEPS", "ovms_cc_library", "ovms_cc_test_library")
 
 COPTS_OV_TRACE = select({
     "//conditions:default": ["-DOV_TRACE=0"],
@@ -27,13 +28,6 @@ LINKOPTS_ADJUSTED = COMMON_STATIC_LIBS_LINKOPTS + select({
     "//conditions:default": [],
     "//:fuzzer_build" : COMMON_FUZZER_LINKOPTS,
 })
-COPTS_CLOUD = select({
-    "//conditions:default": ["-DCLOUD_DISABLE=1"],
-    "//:not_disable_cloud" : ["-DCLOUD_DISABLE=0"],
-})
-
-COPTS_TESTS = COMMON_STATIC_TEST_COPTS + COPTS_CLOUD
-
 
 config_setting(
     name = "windows",
@@ -2031,7 +2025,6 @@ cc_test(
         "test/model_test.cpp",
         "test/model_version_policy_test.cpp",
         "test/modelconfig_test.cpp",
-        "test/modelinstance_test.cpp",
         "test/modelmanager_test.cpp",
         "test/modelversionstatus_test.cpp",
         "test/node_library_manager_test.cpp",
@@ -2264,6 +2257,7 @@ cc_test(
             ":test_test_models",
             ":test_test_models_configs",
             ":test_cmd_exec",
+            ":test_modelinstance_test",
             ] + select({
                 "//conditions:default": [
                     ":openvino_remote_tensors_tests",
@@ -2746,6 +2740,21 @@ cc_library(
     copts = COPTS_TESTS,
 )
 
+ovms_cc_test_library(
+    name = "test_modelinstance_test",
+    srcs = ["test/modelinstance_test.cpp"],
+    deps = [
+        ":test_test_models_configs",
+        ":test_test_with_temp_dir",
+        ":libtest_environment",
+        ":libtest_gpuenvironment",
+        ":modelinstance",
+        ":modelinstanceunloadguard",
+        "//third_party:openvino",
+        "@com_google_googletest//:gtest",
+    ],
+)
+
 cc_library(
     name = "openvino_tests",
     linkstatic = 1,

src/filesystem/BUILD

@@ -13,12 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-load("//:common_settings.bzl", "ovms_cc_library")
-
-COPTS_CLOUD = select({
-    "//conditions:default": ["-DCLOUD_DISABLE=1"],
-    "//:not_disable_cloud" : ["-DCLOUD_DISABLE=0"],
-})
+load("//:common_settings.bzl", "ovms_cc_library", "COPTS_CLOUD")
 
 ovms_cc_library(
     name = "libovmsfilesystem",

src/kfs_frontend/kfs_graph_executor_impl.cpp

@@ -921,14 +921,6 @@ static Status createPacketAndPushIntoGraph(const std::string& name, std::shared_
         return StatusCode::MEDIAPIPE_GRAPH_ADD_PACKET_INPUT_STREAM;
     }
     SPDLOG_DEBUG("Tensor to deserialize:\"{}\"", name);
-    OVMS_RETURN_ON_FAIL(validateRequestCoherencyKFS(*request, request->model_name(), MediapipeGraphDefinition::VERSION));
-    if (!request->raw_input_contents().empty() && (request->raw_input_contents().size() != request->inputs().size())) {
-        std::stringstream ss;
-        ss << "Size of raw_input_contents: " << request->raw_input_contents().size() << " is different than number of inputs: " << request->inputs().size();
-        const std::string details = ss.str();
-        SPDLOG_DEBUG("[servable name: {} version: {}] Invalid message structure - {}", request->model_name(), request->model_version(), details);
-        return Status(StatusCode::INVALID_MESSAGE_STRUCTURE, details);
-    }
     std::unique_ptr<T> inputTensor;
     OVMS_RETURN_ON_FAIL(deserializeTensor(name, *request, inputTensor, pythonBackend));
     SPDLOG_TRACE("Current Timestamp before actual pushing:{}", timestamp.Value());

src/kfs_frontend/kfs_utils.cpp

@@ -23,12 +23,12 @@
 #include <string>
 #include <unordered_map>
 #include <vector>
+#include "src/logging.hpp"
 
-#include "../logging.hpp"
-#include "../profiler.hpp"
-#include "../status.hpp"
-#include "../tensorinfo.hpp"
-#include "../tensor_conversion_common.hpp"
+#include "src/profiler.hpp"
+#include "src/status.hpp"
+#include "src/tensorinfo.hpp"
+#include "src/tensor_conversion_common.hpp"
 
 namespace ovms {
 Precision KFSPrecisionToOvmsPrecision(const KFSDataType& datatype) {
@@ -182,8 +182,15 @@ void setStringPrecision(KFSTensorOutputProto& proto) {
     proto.set_datatype("BYTES");
 }
 
-Status validateRequestCoherencyKFS(const KFSRequest& request, const std::string servableName, model_version_t servableVersion) {
+Status validateRequestCoherencyKFS(const KFSRequest& request, const std::string& servableName, model_version_t servableVersion) {
     if (!request.raw_input_contents().empty()) {
+        if (request.raw_input_contents().size() != request.inputs_size()) {
+            std::stringstream ss;
+            ss << "Size of raw_input_contents: " << request.raw_input_contents().size() << " is different than number of inputs: " << request.inputs_size();
+            const std::string details = ss.str();
+            SPDLOG_DEBUG("[servable name: {} version: {}] Invalid message structure - {}", servableName, servableVersion, details);
+            return Status(StatusCode::INVALID_MESSAGE_STRUCTURE, details);
+        }
         for (auto& input : request.inputs()) {
             if (input.has_contents()) {
                 std::stringstream ss;

src/kfs_frontend/kfs_utils.hpp

@@ -62,7 +62,7 @@ void setStringPrecision(KFSTensorOutputProto& proto);
  * - request.raw_input_content
  * - request.inputs[i].content
  */
-Status validateRequestCoherencyKFS(const KFSRequest& request, const std::string servableName, model_version_t servableVersion);
+Status validateRequestCoherencyKFS(const KFSRequest& request, const std::string& servableName, model_version_t servableVersion);
 size_t getElementsCount(const KFSTensorInputProto& proto, ovms::Precision expectedPrecision);
 int getBinaryInputsSize(const ::KFSRequest::InferInputTensor& tensor);
 class TensorInfo;

src/test/mediapipe_validation_test.cpp

@@ -142,6 +142,12 @@ TEST_F(MediapipeValidationTest, NoDataInRawField) {
     ASSERT_EQ(impl->ModelInfer(nullptr, &request, &response).error_code(), grpc::StatusCode::INVALID_ARGUMENT);
 }
 
+TEST_F(MediapipeValidationTest, RawInputContentsCountMismatch) {
+    prepareDoubleInput();
+    request.mutable_raw_input_contents()->RemoveLast();
+    ASSERT_EQ(impl->ModelInfer(nullptr, &request, &response).error_code(), grpc::StatusCode::INVALID_ARGUMENT);
+}
+
 TEST_F(MediapipeValidationTest, NegativeShape) {
     prepareSingleInput();
     request.mutable_inputs(0)->mutable_shape()->Set(0, -1);

src/test/modelinstance_test.cpp

@@ -27,6 +27,7 @@
 
 #include "../modelinstance.hpp"
 #include "../modelinstanceunloadguard.hpp"
+#include "environment.hpp"
 #include "gpuenvironment.hpp"
 #include "test_models_configs.hpp"
 #include "test_with_temp_dir.hpp"
@@ -705,6 +706,10 @@ TEST_F(TestLoadModel, SuccessfulLoadDummyDimensionRanges) {
 }
 
 TEST_F(TestLoadModel, CorrectNumberOfStreamsSet) {
+// problematic on just one machine in CI
+#if defined(_WIN32)
+    GTEST_SKIP();
+#endif
     ovms::ModelInstance modelInstance("UNUSED_NAME", UNUSED_MODEL_VERSION, *ieCore);
     ovms::ModelConfig config = DUMMY_MODEL_CONFIG;
     config.setPluginConfig({{"NUM_STREAMS", "4"}});

src/test/predict_validation_test.cpp

@@ -571,6 +571,25 @@ TEST_F(KFSPredictValidationInputTensorContent, RequestInputTensorContentAndRawIn
     EXPECT_EQ(status, ovms::StatusCode::INVALID_MESSAGE_STRUCTURE) << status.string();
 }
 
+TEST_F(KFSPredictValidationInputTensorContent, RawInputContentsCountMismatch) {
+    const std::string inputName1 = "input1";
+    const std::string inputName2 = "input2";
+    servableInputs = ovms::tensor_map_t({
+        {inputName1, std::make_shared<ovms::TensorInfo>(inputName1, ovms::Precision::FP32, ovms::shape_t{1, 4}, ovms::Layout{"NC"})},
+        {inputName2, std::make_shared<ovms::TensorInfo>(inputName2, ovms::Precision::FP32, ovms::shape_t{1, 4}, ovms::Layout{"NC"})},
+    });
+    ON_CALL(*instance, getInputsInfo()).WillByDefault(ReturnRef(servableInputs));
+    ON_CALL(*instance, getBatchSize()).WillByDefault(Return(1));
+    ON_CALL(*instance, getModelConfig()).WillByDefault(ReturnRef(modelConfig));
+    preparePredictRequest(request,
+        {{inputName1, std::tuple<ovms::signed_shape_t, ovms::Precision>{{1, 4}, ovms::Precision::FP32}},
+            {inputName2, std::tuple<ovms::signed_shape_t, ovms::Precision>{{1, 4}, ovms::Precision::FP32}}});
+    // Remove one buffer, leaving 1 raw_input_contents for 2 inputs
+    request.mutable_raw_input_contents()->RemoveLast();
+    auto status = instance->mockValidate(&request);
+    EXPECT_EQ(status, ovms::StatusCode::INVALID_MESSAGE_STRUCTURE) << status.string();
+}
+
 TEST_P(KFSPredictValidationInputTensorContent, RequestCorrectContentSizeInputTensorContent) {
     ovms::Precision testedPrecision = GetParam();
     const std::string inputName = "someName";