Skip to content

Commit fc8226d

Browse files
authored
Merge pull request #8455 from nadavMiz/versioning_add_linkat
NSFS | versioning | use linkat in gpfs safe_link to not override existing versions
2 parents cf3b0b7 + 856db75 commit fc8226d

File tree

12 files changed

+105
-33
lines changed

12 files changed

+105
-33
lines changed

.github/workflows/jest-unit-tests.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jobs:
1414

1515
- name: Run Jest Unit Test
1616
run: |
17-
sudo apt-get -y install nasm
17+
sudo apt-get -y install nasm libcap-dev
1818
npm install
1919
npm run build
2020
npm run jest

src/deploy/NVA_build/builder.Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ RUN dnf update -y -q --nobest && \
2121
COPY ./src/deploy/NVA_build/install_arrow_build.sh ./src/deploy/NVA_build/install_arrow_build.sh
2222
ARG BUILD_S3SELECT_PARQUET=0
2323
RUN ./src/deploy/NVA_build/install_arrow_build.sh $BUILD_S3SELECT_PARQUET
24-
RUN dnf install -y -q wget unzip which vim python3 boost-devel && \
24+
RUN dnf install -y -q wget unzip which vim python3 boost-devel libcap-devel && \
2525
dnf group install -y -q "Development Tools" && \
2626
dnf clean all
2727
RUN version="2.15.05" && \

src/deploy/RPM_build/noobaa.spec

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ BuildRequires: python3
3030
BuildRequires: make
3131
BuildRequires: gcc-c++
3232
BuildRequires: boost-devel
33+
BuildRequires: libcap-devel
3334

3435
Recommends: jemalloc
3536

src/native/common.gypi

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
],
4141
'ldflags': [
4242
'-lrt', # librt
43+
'-lcap',
4344
],
4445
}],
4546

src/native/fs/fs_napi.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,7 @@ struct FSWorker : public Napi::AsyncWorker
559559
int _warn_threshold_ms;
560560
double _took_time;
561561
Napi::FunctionReference _report_fs_stats;
562+
bool _should_add_thread_capabilities;
562563

563564
// executes the ctime check in the stat and read file fuctions
564565
// NOTE: If _do_ctime_check = false, then some functions will fallback to using mtime check
@@ -574,6 +575,7 @@ struct FSWorker : public Napi::AsyncWorker
574575
, _errno(0)
575576
, _warn_threshold_ms(0)
576577
, _took_time(0)
578+
, _should_add_thread_capabilities(false)
577579
, _do_ctime_check(false)
578580
{
579581
for (int i = 0; i < (int)info.Length(); ++i) _args_ref.Set(i, info[i]);
@@ -607,6 +609,9 @@ struct FSWorker : public Napi::AsyncWorker
607609
tx.set_user(_uid, _gid);
608610
DBG1("FS::FSWorker::Execute: " << _desc << DVAL(_uid) << DVAL(_gid) << DVAL(geteuid()) << DVAL(getegid()) << DVAL(getuid()) << DVAL(getgid()));
609611

612+
if(_should_add_thread_capabilities) {
613+
tx.add_thread_capabilities();
614+
}
610615
auto start_time = std::chrono::high_resolution_clock::now();
611616
Work();
612617
auto end_time = std::chrono::high_resolution_clock::now();
@@ -641,6 +646,9 @@ struct FSWorker : public Napi::AsyncWorker
641646
{
642647
return gpfs_dl_path != NULL && gpfs_lib_file_exists > -1 && _backend == GPFS_BACKEND;
643648
}
649+
void AddThreadCapabilities() {
650+
_should_add_thread_capabilities = true;
651+
}
644652
virtual void OnOK() override
645653
{
646654
DBG1("FS::FSWorker::OnOK: undefined " << _desc);
@@ -1619,15 +1627,24 @@ struct LinkFileAt : public FSWrapWorker<FileWrap>
16191627
{
16201628
std::string _filepath;
16211629
int _replace_fd;
1630+
bool _should_not_override;
16221631
LinkFileAt(const Napi::CallbackInfo& info)
16231632
: FSWrapWorker<FileWrap>(info)
16241633
, _replace_fd(-1)
1634+
, _should_not_override(false)
16251635
{
16261636
_filepath = info[1].As<Napi::String>();
16271637
if (info.Length() > 2 && !info[2].IsUndefined()) {
16281638
_replace_fd = info[2].As<Napi::Number>();
16291639
}
1630-
Begin(XSTR() << "LinkFileAt " << DVAL(_wrap->_path) << DVAL(_wrap->_fd) << DVAL(_filepath));
1640+
if (info.Length() > 3 && !info[3].IsUndefined()) {
1641+
_should_not_override = info[3].As<Napi::Boolean>();
1642+
}
1643+
if(_replace_fd < 0 && _should_not_override) {
1644+
//set thread capabilities to allow linkat from user other than root.
1645+
AddThreadCapabilities();
1646+
}
1647+
Begin(XSTR() << "LinkFileAt " << DVAL(_wrap->_path) << DVAL(_wrap->_fd) << DVAL(_filepath) << DVAL(_should_not_override));
16311648
}
16321649
virtual void Work()
16331650
{
@@ -1638,6 +1655,8 @@ struct LinkFileAt : public FSWrapWorker<FileWrap>
16381655
// Linux will fail the linkat() if the file already exist and we want to replace it if it existed.
16391656
if (_replace_fd >= 0) {
16401657
SYSCALL_OR_RETURN(dlsym_gpfs_linkatif(fd, "", AT_FDCWD, _filepath.c_str(), AT_EMPTY_PATH, _replace_fd));
1658+
} else if (_should_not_override){
1659+
SYSCALL_OR_RETURN(linkat(fd, "", AT_FDCWD, _filepath.c_str(), AT_EMPTY_PATH));
16411660
} else {
16421661
SYSCALL_OR_RETURN(dlsym_gpfs_linkat(fd, "", AT_FDCWD, _filepath.c_str(), AT_EMPTY_PATH));
16431662
}

src/native/util/os.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ class ThreadScope
3838
change_user();
3939
}
4040

41+
int add_thread_capabilities();
42+
4143
const static uid_t orig_uid;
4244
const static gid_t orig_gid;
4345
const static std::vector<gid_t> orig_groups;

src/native/util/os_darwin.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,14 @@ ThreadScope::restore_user()
5454
}
5555
}
5656

57+
int
58+
ThreadScope::add_thread_capabilities()
59+
{
60+
//set capabilities not used in darwin
61+
LOG("function set_capabilities_linkat is unsupported in darwin");
62+
return -1;
63+
}
64+
5765
} // namespace noobaa
5866

5967
#endif

src/native/util/os_linux.cpp

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include <sys/syscall.h>
77
#include <sys/types.h>
8+
#include <sys/capability.h>
89
#include <grp.h>
910
#include <limits.h>
1011

@@ -35,7 +36,7 @@ const std::vector<gid_t> ThreadScope::orig_groups = [] {
3536

3637
/**
3738
* set the effective uid/gid of the current thread using direct syscalls
38-
* unsets the supplementary group IDs for the current thread using direct syscall
39+
* unsets the supplementary group IDs for the current thread using direct syscall
3940
* we have to bypass the libc wrappers because posix requires it to syncronize
4041
* uid & gid to all threads which is undesirable in our case.
4142
*/
@@ -64,6 +65,41 @@ ThreadScope::restore_user()
6465
}
6566
}
6667

68+
int
69+
ThreadScope::add_thread_capabilities() {
70+
cap_t caps = cap_get_proc();
71+
cap_flag_value_t cap_flag_value;
72+
if(caps == NULL) {
73+
LOG("ThreadScope::set_thread_capabilities - cap_get_proc failed");
74+
return -1;
75+
}
76+
if(cap_get_flag(caps, CAP_DAC_READ_SEARCH, CAP_EFFECTIVE, &cap_flag_value) < 0) {
77+
LOG("ThreadScope::set_thread_capabilities - cap_get_flag failed");
78+
cap_free(caps);
79+
return -1;
80+
}
81+
if(cap_flag_value == CAP_SET) {
82+
LOG("ThreadScope::cap_flag_value - capability already exists");
83+
cap_free(caps);
84+
return 0;
85+
}
86+
cap_value_t newcaps[1] = { CAP_DAC_READ_SEARCH, };
87+
if(cap_set_flag(caps, CAP_EFFECTIVE, 1, newcaps, CAP_SET) < 0) {
88+
LOG("ThreadScope::set_thread_capabilities - cap_set_flag failed");
89+
cap_free(caps);
90+
return -1;
91+
}
92+
if(cap_set_proc(caps) < 0) {
93+
LOG("ThreadScope::set_thread_capabilities - cap_set_proc failed");
94+
cap_free(caps);
95+
return -1;
96+
}
97+
if(cap_free(caps) < 0) {
98+
LOG("cap_free failed");
99+
}
100+
return 0;
101+
}
102+
67103
} // namespace noobaa
68104

69105
#endif

src/sdk/namespace_fs.js

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,15 +1473,23 @@ class NamespaceFS {
14731473
let retries = config.NSFS_RENAME_RETRIES;
14741474
for (;;) {
14751475
try {
1476-
const new_ver_info = !is_gpfs && await this._get_version_info(fs_context, new_ver_tmp_path);
1477-
// get latest version_id if exists
1478-
const latest_ver_info = await this._get_version_info(fs_context, latest_ver_path);
1479-
const versioned_path = latest_ver_info && this._get_version_path(key, latest_ver_info.version_id_str);
1480-
const versioned_info = latest_ver_info && await this._get_version_info(fs_context, versioned_path);
1481-
1482-
gpfs_options = await this._open_files_gpfs(fs_context, new_ver_tmp_path, latest_ver_path, upload_file,
1483-
latest_ver_info, open_mode, undefined, versioned_info);
1476+
let new_ver_info;
1477+
let latest_ver_info;
1478+
if (is_gpfs) {
1479+
const latest_ver_info_exist = await native_fs_utils.is_path_exists(fs_context, latest_ver_path);
1480+
gpfs_options = await this._open_files_gpfs(fs_context, new_ver_tmp_path, latest_ver_path, upload_file,
1481+
latest_ver_info_exist, open_mode, undefined, undefined);
1482+
1483+
//get latest version if exists
1484+
const latest_fd = gpfs_options?.move_to_dst?.dst_file;
1485+
latest_ver_info = latest_fd && await this._get_version_info(fs_context, undefined, latest_fd);
1486+
} else {
1487+
new_ver_info = await this._get_version_info(fs_context, new_ver_tmp_path);
1488+
//get latest version if exists. TODO use fd like in GPFS
1489+
latest_ver_info = await this._get_version_info(fs_context, latest_ver_path);
1490+
}
14841491
const bucket_tmp_dir_path = this.get_bucket_tmpdir_full_path();
1492+
const versioned_path = latest_ver_info && this._get_version_path(key, latest_ver_info.version_id_str);
14851493
dbg.log1('Namespace_fs._move_to_dest_version:', latest_ver_info, new_ver_info, gpfs_options);
14861494

14871495
if (this._is_versioning_suspended()) {
@@ -2725,10 +2733,12 @@ class NamespaceFS {
27252733
// path - specifies the path to version
27262734
// if version xattr contains version info - return info by xattr
27272735
// else - it's a null version - return stat
2728-
async _get_version_info(fs_context, version_path) {
2736+
// if fd is passed, will use fd instead of path to stat
2737+
async _get_version_info(fs_context, version_path, fd) {
27292738
try {
2730-
const stat = await nb_native().fs.stat(fs_context, version_path, { skip_user_xattr: true });
2731-
dbg.log1('NamespaceFS._get_version_info stat ', stat, version_path);
2739+
const stat = fd ? await fd.stat(fs_context, { skip_user_xattr: true }) :
2740+
await nb_native().fs.stat(fs_context, version_path, { skip_user_xattr: true });
2741+
dbg.log1('NamespaceFS._get_version_info stat ', stat, version_path, fd);
27322742

27332743
const version_id_str = this._get_version_id_by_xattr(stat);
27342744
const ver_info_by_xattr = this._extract_version_info_from_xattr(version_id_str);
@@ -3152,8 +3162,8 @@ class NamespaceFS {
31523162
// opens the unopened files involved in the version move during upload/deletion
31533163
// returns an object contains the relevant options for the move/unlink flow
31543164
// eslint-disable-next-line max-params
3155-
async _open_files_gpfs(fs_context, src_path, dst_path, upload_or_dir_file, dst_ver_info, open_mode, delete_version, versioned_info) {
3156-
dbg.log1('Namespace_fs._open_files_gpfs:', src_path, src_path && path.dirname(src_path), dst_path, upload_or_dir_file, dst_ver_info, open_mode, delete_version, versioned_info);
3165+
async _open_files_gpfs(fs_context, src_path, dst_path, upload_or_dir_file, dst_ver_exist, open_mode, delete_version, versioned_info) {
3166+
dbg.log1('Namespace_fs._open_files_gpfs:', src_path, src_path && path.dirname(src_path), dst_path, upload_or_dir_file, Boolean(dst_ver_exist), open_mode, delete_version, versioned_info);
31573167
const is_gpfs = native_fs_utils._is_gpfs(fs_context);
31583168
if (!is_gpfs) return;
31593169

@@ -3179,13 +3189,13 @@ class NamespaceFS {
31793189
src_file = upload_or_dir_file;
31803190
dir_file = await native_fs_utils.open_file(fs_context, this.bucket_path, path.dirname(src_path), 'r');
31813191
}
3182-
if (dst_ver_info) {
3192+
if (dst_ver_exist) {
31833193
dbg.log1('NamespaceFS._open_files_gpfs dst version exist - opening dst version file...');
31843194
dst_file = await native_fs_utils.open_file(fs_context, this.bucket_path, dst_path, 'r');
31853195
}
31863196
return {
3187-
move_to_versions: { src_file: dst_file, dir_file, should_override: false },
3188-
move_to_dst: { src_file, dst_file, dir_file, versioned_file }
3197+
move_to_versions: { src_file: dst_file, dir_file },
3198+
move_to_dst: { src_file, dst_file, dir_file}
31893199
};
31903200
} catch (err) {
31913201
dbg.error('NamespaceFS._open_files_gpfs couldn\'t open files', err);

src/sdk/nb.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1000,7 +1000,7 @@ interface NativeFile {
10001000
write(fs_context: NativeFSContext, buffer: Buffer, len: number, offset?: number): Promise<void>;
10011001
writev(fs_context: NativeFSContext, buffers: Buffer[], offset?: number): Promise<void>;
10021002
replacexattr(fs_context: NativeFSContext, xattr: NativeFSXattr, clear_prefix?: string): Promise<void>;
1003-
linkfileat(fs_context: NativeFSContext, path: string, fd?: number): Promise<void>;
1003+
linkfileat(fs_context: NativeFSContext, path: string, fd?: number, should_not_override?: boolean): Promise<void>;
10041004
fsync(fs_context: NativeFSContext): Promise<void>;
10051005
fd: number;
10061006
flock(fs_context: NativeFSContext, operation: "EXCLUSIVE" | "SHARED" | "UNLOCK"): Promise<void>;

0 commit comments

Comments
 (0)