Integration #145

al1img · 2026-01-23T14:04:21Z

No description provided.

mlohvynenko

Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>

mykola-kobets-epam

Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>

Copilot

Pull request overview

This PR updates several components for “integration” behavior changes, including making container runtime start/stop operations idempotent, refactoring SMController blob URL lookups to use an item info provider, and reworking Fileserver URL handling around Poco::URI.

Changes:

Container runtime: StartInstance/StopInstance now return success for “already running” / “not running” cases (idempotent behavior).
Fileserver: store a parsed Poco::URI and generate output URLs using a root-relative filesystem path.
SMController: replace BlobInfoProviderItf usage with ItemInfoProviderItf for GetBlobsInfos, update app wiring and tests/stubs accordingly.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/sm/launcher/runtimes/container/container.cpp	Makes start/stop idempotent; adds logic to reuse/stop existing instance before starting
src/sm/launcher/runtimes/container/tests/container.cpp	Updates expectations for idempotent start/stop behavior
src/common/fileserver/fileserver.cpp	Switches to `Poco::URI` member; changes URL translation logic and server socket port retrieval
src/common/fileserver/fileserver.hpp	Stores `Poco::URI` instead of host/port; adjusts includes
src/common/fileserver/tests/fileserver.cpp	Updates TranslateFilePathURL test input to match new translation logic
src/cm/smcontroller/smcontroller.cpp	Implements `GetBlobsInfos` via `ItemInfoProviderItf::GetBlobURL`
src/cm/smcontroller/smcontroller.hpp	Updates SMController init signature/member from blob info provider to item info provider
src/cm/smcontroller/tests/smcontroller.cpp	Updates tests to use the new item info provider stub
src/cm/smcontroller/tests/stubs/iteminfoproviderstub.hpp	Adds stub for `ItemInfoProviderItf` used by tests
src/cm/smcontroller/tests/stubs/blobinfoproviderstub.hpp	Removes blob info provider stub
src/cm/app/aoscore.cpp	Wires SMController init to pass `mImageManager` as item info provider

Comments suppressed due to low confidence (1)

src/sm/launcher/runtimes/container/container.cpp:182

When an instance already exists but isn’t active, this code stops it but never removes/replaces the existing entry in mCurrentInstances. Because mCurrentInstances.insert(...) won’t overwrite an existing key, the new Instance can fail to be tracked while the map still points to the old (stopped) instance. Use erase(it) before creating the new instance, or insert_or_assign/operator[] to replace the entry (and ensure the old instance is properly removed).

            instance = std::make_shared<Instance>(instanceInfo, mConfig, mNodeInfo, *mFileSystem, *mRunner,
                *mMonitoring, *mItemInfoProvider, *mNetworkManager, *mPermHandler, *mResourceInfoProvider, *mOCISpec);

            mCurrentInstances.insert({static_cast<const InstanceIdent&>(instanceInfo), instance});
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-23T14:12:58Z

src/sm/launcher/runtimes/container/container.cpp

+            if (instance) {
+                if (auto err = instance->Stop(); !err.IsNone()) {
+                    LOG_ERR() << "Failed to stop instance"
+                              << Log::Field("instance", static_cast<const InstanceIdent&>(instanceInfo))
+                              << Log::Field(err);
+                }
+
+                instance->GetStatus(status);
+                SendInstanceStatus(status);
            }


instance->Stop() (and SendInstanceStatus) is executed while holding mMutex. Instance::Stop() performs blocking work (runner stop, FS ops, network cleanup), so doing it under the runtime mutex can stall other operations and risks lock-order deadlocks. Prefer the same pattern as StopInstance: capture+erase under mMutex, release the lock, then stop/send status outside the critical section (and consider propagating stop failures instead of only logging).

Copilot · 2026-01-23T14:12:58Z

src/common/fileserver/fileserver.cpp

        mRootDir = rootDir;
-        Poco::URI uri(serverURL);
+        mURI     = serverURL.c_str();

-        mHost = uri.getHost();
-        mPort = uri.getPort();
+        if (mURI.getScheme().empty()) {
+            mURI.setScheme("http");
+        }

-        if (mHost.empty()) {
-            mHost = "localhost";
+        if (mURI.getHost().empty()) {
+            mURI.setHost("localhost");
        }

-        if (mPort == 0) {
-            mPort = cDefaultPort;
+        if (mURI.getPort() == 0) {
+            mURI.setPort(cDefaultPort);


Poco::URI parsing here can mis-handle config-style values like "localhost:8080" (common in this repo, e.g. cm/config/tests/config.cpp expects fileServerUrl without a scheme). For such inputs Poco treats localhost as the URI scheme, so mURI.getScheme() won’t be empty and you won’t default to http, producing invalid URLs (e.g. localhost://localhost:8080/...). Consider normalizing serverURL by prepending http:// when it lacks ://, or explicitly handling the host:port form before assigning to mURI.

Copilot · 2026-01-23T14:12:58Z

src/common/fileserver/fileserver.cpp

+        uri.setPath(fs::relative(fs::path(filePath.CStr()), mRootDir).string());



fs::relative(fs::path(filePath), mRootDir) can yield paths containing .. when filePath is outside mRootDir, and this URL path is later used by the HTTP handler without any traversal checks. This can generate URLs that allow reading files outside the intended root directory. Validate that filePath resolves within mRootDir (e.g., compare canonical/weakly_canonical paths) and reject or sanitize any relative path containing ../absolute components before setting it on the URI.

Suggested change

uri.setPath(fs::relative(fs::path(filePath.CStr()), mRootDir).string());

// Canonicalize root and target paths to ensure containment.

const fs::path rootCanonical = fs::weakly_canonical(fs::path(mRootDir));

const fs::path fileCanonical = fs::weakly_canonical(fs::path(filePath.CStr()));

// Verify that fileCanonical is within rootCanonical.

auto rootIt = rootCanonical.begin();

auto fileIt = fileCanonical.begin();

for (; rootIt != rootCanonical.end() && fileIt != fileCanonical.end(); ++rootIt, ++fileIt) {

if (*rootIt != *fileIt) {

break;

}

}

if (rootIt != rootCanonical.end()) {

throw std::runtime_error("filePath is outside of the fileserver root directory");

}

fs::path relativePath = fs::relative(fileCanonical, rootCanonical);

// Sanity checks: the URL path must not be absolute or contain any ".." components.

if (relativePath.is_absolute()) {

throw std::runtime_error("relative path for URL must not be absolute");

}

for (const auto& part : relativePath) {

if (part == "..") {

throw std::runtime_error("relative path for URL must not contain '..' components");

}

}

uri.setPath(relativePath.string());

Copilot · 2026-01-23T14:12:59Z

src/cm/smcontroller/smcontroller.cpp

-    for (const auto& blobInfo : blobsInfo) {
-        if (blobInfo.mURLs.Size() != 1) {
-            return grpc::Status(grpc::StatusCode::NOT_FOUND, "blob URL not found");
+        if (auto err = mItemInfoProvider->GetBlobURL(request->digests(i).c_str(), digestURL); !err.IsNone()) {


GetBlobURL errors are always mapped to grpc::StatusCode::INTERNAL. This changes prior behavior where missing blob info/URL resulted in NOT_FOUND. If GetBlobURL returns ErrorEnum::eNotFound, return grpc::StatusCode::NOT_FOUND (and keep INTERNAL for unexpected failures) so clients can distinguish missing data from server errors.

Suggested change

if (auto err = mItemInfoProvider->GetBlobURL(request->digests(i).c_str(), digestURL); !err.IsNone()) {

if (auto err = mItemInfoProvider->GetBlobURL(request->digests(i).c_str(), digestURL); !err.IsNone()) {

if (err == ErrorEnum::eNotFound) {

return grpc::Status(grpc::StatusCode::NOT_FOUND, err.Message());

}

MykolaSuperman

Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com> Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>

Poco::URI returns empty host and port if scheme is not present in str uri. Add checking URI scheme and use Poco::URI for creating file URL and file server itself. Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com> Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>

SM launcher may ask to start/stop already started/stopped instances. Runtime should not return error in this case. Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com> Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>

codecov · 2026-01-23T15:56:37Z

Codecov Report

❌ Patch coverage is 69.23077% with 16 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (feature_unification@808e03a). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
.../smcontroller/tests/stubs/iteminfoproviderstub.hpp	46.15%	7 Missing ⚠️
src/sm/launcher/runtimes/container/container.cpp	66.66%	5 Missing ⚠️
src/common/fileserver/fileserver.cpp	76.92%	3 Missing ⚠️
src/cm/smcontroller/smcontroller.cpp	85.71%	1 Missing ⚠️

Additional details and impacted files

@@                  Coverage Diff                   @@
##             feature_unification     #145   +/-   ##
======================================================
  Coverage                       ?   81.98%           
======================================================
  Files                          ?      303           
  Lines                          ?    28286           
  Branches                       ?     3015           
======================================================
  Hits                           ?    23189           
  Misses                         ?     5097           
  Partials                       ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sonarqubecloud · 2026-01-23T16:04:15Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot AI review requested due to automatic review settings January 23, 2026 14:04

Copilot started reviewing on behalf of al1img January 23, 2026 14:04 View session

mlohvynenko approved these changes Jan 23, 2026

View reviewed changes

mykola-kobets-epam approved these changes Jan 23, 2026

View reviewed changes

Copilot AI reviewed Jan 23, 2026

View reviewed changes

MykolaSuperman approved these changes Jan 23, 2026

View reviewed changes

al1img added 3 commits January 23, 2026 17:30

al1img force-pushed the integration branch from 1af1307 to 3142b91 Compare January 23, 2026 15:30

al1img merged commit f4b7e43 into aosedge:feature_unification Jan 23, 2026
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integration #145

Integration #145

Uh oh!

al1img commented Jan 23, 2026

Uh oh!

mlohvynenko left a comment

Uh oh!

mykola-kobets-epam left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

Copilot AI Jan 23, 2026

Uh oh!

MykolaSuperman left a comment

Uh oh!

codecov bot commented Jan 23, 2026

Uh oh!

sonarqubecloud bot commented Jan 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

		uri.setPath(fs::relative(fs::path(filePath.CStr()), mRootDir).string());

-        uri.setPath(fs::relative(fs::path(filePath.CStr()), mRootDir).string());
+        // Canonicalize root and target paths to ensure containment.
+        const fs::path rootCanonical = fs::weakly_canonical(fs::path(mRootDir));
+        const fs::path fileCanonical = fs::weakly_canonical(fs::path(filePath.CStr()));
+        // Verify that fileCanonical is within rootCanonical.
+        auto rootIt = rootCanonical.begin();
+        auto fileIt = fileCanonical.begin();
+        for (; rootIt != rootCanonical.end() && fileIt != fileCanonical.end(); ++rootIt, ++fileIt) {
+            if (*rootIt != *fileIt) {
+                break;
+            }
+        }
+        if (rootIt != rootCanonical.end()) {
+            throw std::runtime_error("filePath is outside of the fileserver root directory");
+        }
+        fs::path relativePath = fs::relative(fileCanonical, rootCanonical);
+        // Sanity checks: the URL path must not be absolute or contain any ".." components.
+        if (relativePath.is_absolute()) {
+            throw std::runtime_error("relative path for URL must not be absolute");
+        }
+        for (const auto& part : relativePath) {
+            if (part == "..") {
+                throw std::runtime_error("relative path for URL must not contain '..' components");
+            }
+        }
+        uri.setPath(relativePath.string());

Integration #145

Integration #145

Uh oh!

Conversation

al1img commented Jan 23, 2026

Uh oh!

mlohvynenko left a comment

Choose a reason for hiding this comment

Uh oh!

mykola-kobets-epam left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

MykolaSuperman left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jan 23, 2026

Codecov Report

Uh oh!

sonarqubecloud bot commented Jan 23, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants