Skip to content

Commit

Permalink
feat: Disable logging (stdout, stderr) by default
Browse files Browse the repository at this point in the history
Bug: b/369223425
Change-Id: Ibe9d20c06dc4bb20409551ce83b0b638c518a915
GitOrigin-RevId: 79a661d6c545149b52cfcd3c646e1503877f52f3
  • Loading branch information
Privacy Sandbox Team authored and copybara-github committed Sep 25, 2024
1 parent 99e1450 commit 74db753
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 5 deletions.
26 changes: 21 additions & 5 deletions src/roma/byob/container/run_workers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ struct WorkerImplArg {
int fd;
std::string_view code_token;
std::string_view binary_path;
const int dev_null_fd;
};

int WorkerImpl(void* arg) {
Expand Down Expand Up @@ -98,6 +99,10 @@ int WorkerImpl(void* arg) {
PCHECK(::mount(binary_dir.c_str(), target.c_str(), nullptr, MS_BIND,
nullptr) == 0);
}
{
PCHECK(::dup2(worker_impl_arg.dev_null_fd, STDOUT_FILENO) != -1);
PCHECK(::dup2(worker_impl_arg.dev_null_fd, STDERR_FILENO) != -1);
}

// MS_REC needed here to get other mounts (/lib, /lib64 etc)
PCHECK(::mount(worker_impl_arg.pivot_root_dir.data(),
Expand Down Expand Up @@ -144,7 +149,8 @@ struct PidAndPivotRootDir {
// all cases, this is because Roma is shutting down and is not an error.
std::optional<PidAndPivotRootDir> ConnectSendCloneAndExec(
absl::Span<const std::string> mounts, std::string_view socket_name,
std::string_view code_token, std::string_view binary_path) {
std::string_view code_token, std::string_view binary_path,
const int dev_null_fd) {
const int fd = ::socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
if (fd == -1) {
PLOG(ERROR) << "socket()";
Expand All @@ -171,6 +177,7 @@ std::optional<PidAndPivotRootDir> ConnectSendCloneAndExec(
.fd = fd,
.code_token = code_token,
.binary_path = binary_path,
.dev_null_fd = dev_null_fd,
};

// Explicitly 16-byte align the stack. Otherwise, `clone` on aarch64 may hang
Expand Down Expand Up @@ -225,6 +232,11 @@ int main(int argc, char** argv) {
PLOG(ERROR) << "connect() to " << socket_name << " failed";
return -1;
}
const int dev_null_fd = open("/dev/null", O_WRONLY);
if (dev_null_fd < 0) {
PLOG(ERROR) << "open failed for /dev/null";
return -1;
}
struct UdfInstanceMetadata {
std::string pivot_root_dir;
std::string code_token;
Expand All @@ -234,7 +246,8 @@ int main(int argc, char** argv) {
absl::Mutex mu;
absl::flat_hash_map<int, UdfInstanceMetadata> pid_to_udf; // Guarded by mu.
bool shutdown = false; // Guarded by mu.
std::thread reloader([&socket_name, &mounts, &mu, &pid_to_udf, &shutdown] {
std::thread reloader([&socket_name, &mounts, &mu, &pid_to_udf, &shutdown,
&dev_null_fd] {
{
auto fn = [&] {
mu.AssertReaderHeld();
Expand Down Expand Up @@ -279,7 +292,7 @@ int main(int argc, char** argv) {
// Start a new worker with the same UDF as the most-recently ended worker.
std::optional<PidAndPivotRootDir> pid_and_pivot_root_dir =
ConnectSendCloneAndExec(mounts, socket_name, udf.code_token,
udf.binary_path);
udf.binary_path, dev_null_fd);
CHECK(std::filesystem::remove_all(udf.pivot_root_dir));
if (!pid_and_pivot_root_dir.has_value()) {
return;
Expand Down Expand Up @@ -311,6 +324,9 @@ int main(int argc, char** argv) {
LOG(ERROR) << "Failed to remove " << udf.pivot_root_dir << ": " << ec;
}
}
if (::close(dev_null_fd) == -1) {
PLOG(ERROR) << "close(" << dev_null_fd << ")";
}
};
FileInputStream input(fd);
while (true) {
Expand Down Expand Up @@ -353,7 +369,7 @@ int main(int argc, char** argv) {
for (int i = 0; i < request.n_workers() - 1; ++i) {
std::optional<PidAndPivotRootDir> pid_and_pivot_root_dir =
ConnectSendCloneAndExec(mounts, socket_name, request.code_token(),
binary_path.native());
binary_path.native(), dev_null_fd);
if (!pid_and_pivot_root_dir.has_value()) {
return -1;
}
Expand All @@ -369,7 +385,7 @@ int main(int argc, char** argv) {
// Start n-th worker out of loop.
std::optional<PidAndPivotRootDir> pid_and_pivot_root_dir =
ConnectSendCloneAndExec(mounts, socket_name, request.code_token(),
binary_path.native());
binary_path.native(), dev_null_fd);
if (!pid_and_pivot_root_dir.has_value()) {
return -1;
}
Expand Down
19 changes: 19 additions & 0 deletions src/roma/byob/test/roma_byob_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ const std::filesystem::path kUdfPath = "/udf";
const std::filesystem::path kGoLangBinaryFilename = "sample_go_udf";
const std::filesystem::path kCPlusPlusBinaryFilename = "sample_udf";
const std::filesystem::path kCPlusPlusNewBinaryFilename = "new_udf";
const std::filesystem::path kCPlusPlusLogBinaryFilename = "log_udf";
const std::filesystem::path kCallbackPayloadReadUdfFilename =
"callback_payload_read_udf";
constexpr std::string_view kFirstUdfOutput = "Hello, world!";
constexpr std::string_view kNewUdfOutput = "I am a new UDF!";
constexpr std::string_view kGoBinaryOutput = "Hello, world from Go!";
constexpr std::string_view kLogUdfOutput = "I am a UDF that logs.";

SampleResponse SendRequestAndGetResponse(
ByobSampleService<>& roma_service, std::string_view code_token,
Expand Down Expand Up @@ -280,5 +282,22 @@ TEST(RomaByobTest, ExecuteCppBinaryWithHostCallbackInSandboxMode) {
ASSERT_TRUE(response.ok());
EXPECT_EQ((*response)->payload_size(), payload_size);
}

TEST(RomaByobTest, VerifyNoStdOutStdErrEgressionByDefault) {
ByobSampleService<> roma_service = GetRomaService(Mode::kModeSandbox,
/*num_workers=*/1);

std::string code_token =
LoadCode(roma_service, kUdfPath / kCPlusPlusLogBinaryFilename);

::testing::internal::CaptureStdout();
::testing::internal::CaptureStderr();
EXPECT_THAT(SendRequestAndGetResponse(roma_service, code_token).greeting(),
::testing::StrEq(kLogUdfOutput));
std::string stdout_collected = ::testing::internal::GetCapturedStdout();
std::string stderr_collected = ::testing::internal::GetCapturedStderr();
EXPECT_THAT(stdout_collected, ::testing::IsEmpty());
EXPECT_THAT(stderr_collected, ::testing::IsEmpty());
}
} // namespace
} // namespace privacy_sandbox::server_common::byob::test
11 changes: 11 additions & 0 deletions src/roma/byob/udf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,22 @@ roma_host_api_cc_library(
deps = ["//src/roma/byob/udf:sample_service_native_functions"],
)

cc_binary(
name = "log_udf",
srcs = ["log_udf.cc"],
visibility = ["//visibility:public"],
deps = [
":sample_cc_proto",
"@com_google_protobuf//:protobuf",
],
)

filegroup(
name = "udf_binaries",
srcs = [
":callback_payload_read_udf",
":callback_payload_write_udf",
":log_udf",
":new_udf",
":payload_read_udf",
":payload_write_udf",
Expand Down
51 changes: 51 additions & 0 deletions src/roma/byob/udf/log_udf.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2024 Google LLC
//
// Licensed 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 <iostream>

#include "google/protobuf/any.pb.h"
#include "google/protobuf/util/delimited_message_util.h"
#include "src/roma/byob/udf/sample.pb.h"

using ::google::protobuf::io::FileInputStream;
using ::google::protobuf::util::ParseDelimitedFromZeroCopyStream;
using ::google::protobuf::util::SerializeDelimitedToFileDescriptor;
using ::privacy_sandbox::server_common::byob::SampleResponse;

void ReadRequestFromFd(int fd) {
google::protobuf::Any bin_request;
FileInputStream input(fd);
ParseDelimitedFromZeroCopyStream(&bin_request, &input, nullptr);
}

void WriteResponseToFd(int fd, SampleResponse resp) {
google::protobuf::Any any;
any.PackFrom(std::move(resp));
google::protobuf::util::SerializeDelimitedToFileDescriptor(any, fd);
}

int main(int argc, char* argv[]) {
std::cout << "I am a stdout log." << std::endl;
std::cerr << "I am a stderr log." << std::endl;
if (argc < 2) {
std::cerr << "Not enough arguments!" << std::endl;
return -1;
}
int fd = std::stoi(argv[1]);
ReadRequestFromFd(fd);
SampleResponse bin_response;
bin_response.set_greeting("I am a UDF that logs.");
WriteResponseToFd(fd, std::move(bin_response));
return 0;
}

0 comments on commit 74db753

Please sign in to comment.