Skip to content

Commit a383420

Browse files
guymguymguymguym
authored
Merge pull request #8799 from guymguym/guy-md-conditions-nsfs
NSFS check_md_conditions for GET/HEAD (not yet for PUT/DELETE)
2 parents e1e56a8 + 0e9288b commit a383420

File tree

11 files changed

+87
-76
lines changed

11 files changed

+87
-76
lines changed

‎docs/dev_guide/ceph_s3_tests/ceph_s3_tests_pending_list_status.md

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,6 @@ Attached a table with tests that where investigated and their status (this table
4646
| test_object_create_bad_date_none_aws2 | Internal Component | [438](https://github.com/ceph/s3-tests/issues/438) | It used to pass in the past (not related to code change in our repo) |
4747
| test_bucket_create_bad_authorization_invalid_aws2 | Internal Component | [438](https://github.com/ceph/s3-tests/issues/438) | It used to pass in the past (not related to code change in our repo) |
4848
| test_bucket_create_bad_date_none_aws2 | Internal Component | [438](https://github.com/ceph/s3-tests/issues/438) | It used to pass in the past (not related to code change in our repo) |
49-
| test_get_object_ifnonematch_good | Internal Component | | It used to pass in the past (not related to code
50-
change in our repo) - stopped passing between the update of commit hash 6861c3d81081a6883fb90d66cb60392e1abdf3ca to da91ad8bbf899c72199df35b69e9393c706aabee |
51-
| test_get_object_ifmodifiedsince_failed | Internal Component | | It used to pass in the past (not related to code
52-
change in our repo) - stopped passing between the update of commit hash 6861c3d81081a6883fb90d66cb60392e1abdf3ca to da91ad8bbf899c72199df35b69e9393c706aabee |
5349
| test_versioning_concurrent_multi_object_delete | Faulty Test | [588](https://github.com/ceph/s3-tests/issues/588) |
5450
| test_get_bucket_encryption_s3 | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) |
5551
| test_get_bucket_encryption_kms | Faulty Test | [613](https://github.com/ceph/s3-tests/issues/613) |

‎src/endpoint/s3/s3_errors.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,8 @@ S3Error.AccessControlListNotSupported = Object.freeze({
481481
/////////////////////////////////////
482482
S3Error.NotModified = Object.freeze({
483483
code: 'NotModified',
484-
message: 'The resource was not modified according to the conditions in the provided headers.',
484+
// this short undescriptive message is compatible with AWS and is expected by s3-tests
485+
message: 'Not Modified',
485486
http_code: 304,
486487
});
487488
S3Error.BadRequest = Object.freeze({

‎src/endpoint/s3/s3_rest.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ function _prepare_error(req, res, err) {
450450
if (res.headersSent) {
451451
dbg.log0('Sent reply in body, bit too late for Etag header');
452452
} else {
453-
res.setHeader('ETag', err.rpc_data.etag);
453+
res.setHeader('ETag', '"' + err.rpc_data.etag + '"');
454454
}
455455
}
456456
if (err.rpc_data.last_modified) {

‎src/native/util/os_darwin.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ get_supplemental_groups_by_uid(uid_t uid, std::vector<gid_t>& groups)
5252
struct passwd* pw = getpwuid(uid);
5353
if (pw == NULL) {
5454
if (errno == 0) {
55-
LOG("get_supplemental_groups_by_uid: no record for uid " << uid);
55+
// LOG("get_supplemental_groups_by_uid: no record for uid " << uid);
5656
} else {
5757
LOG("WARNING: get_supplemental_groups_by_uid: getpwuid failed: " << strerror(errno));
5858
}

‎src/native/util/os_linux.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ get_supplemental_groups_by_uid(uid_t uid, std::vector<gid_t>& groups)
3838
struct passwd* pw = getpwuid(uid);
3939
if (pw == NULL) {
4040
if (errno == 0) {
41-
LOG("get_supplemental_groups_by_uid: no record for uid " << uid);
41+
// LOG("get_supplemental_groups_by_uid: no record for uid " << uid);
4242
} else {
4343
LOG("WARNING: get_supplemental_groups_by_uid: getpwuid failed: " << strerror(errno));
4444
}

‎src/sdk/namespace_fs.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const error_utils = require('../util/error_utils');
1818
const stream_utils = require('../util/stream_utils');
1919
const buffer_utils = require('../util/buffer_utils');
2020
const size_utils = require('../util/size_utils');
21+
const http_utils = require('../util/http_utils');
2122
const native_fs_utils = require('../util/native_fs_utils');
2223
const FileWriter = require('../util/file_writer');
2324
const LRUCache = require('../util/lru_cache');
@@ -970,6 +971,10 @@ class NamespaceFS {
970971
}
971972
}
972973
this._throw_if_delete_marker(stat, params);
974+
http_utils.check_md_conditions(params.md_conditions, {
975+
etag: this._get_etag(stat),
976+
last_modified_time: stat.mtime,
977+
});
973978
return this._get_object_info(params.bucket, params.key, stat, isDir);
974979
} catch (err) {
975980
if (this._should_update_issues_report(params, file_path, err)) {
@@ -1045,6 +1050,10 @@ class NamespaceFS {
10451050
}
10461051
this._throw_if_delete_marker(stat, params);
10471052
// await this._fail_if_archived_or_sparse_file(fs_context, file_path, stat);
1053+
http_utils.check_md_conditions(params.md_conditions, {
1054+
etag: this._get_etag(stat),
1055+
last_modified_time: stat.mtime,
1056+
});
10481057

10491058
const start = Number(params.start) || 0;
10501059
const end = isNaN(Number(params.end)) ? Infinity : Number(params.end);

‎src/server/object_services/object_server.js

Lines changed: 13 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -791,7 +791,7 @@ async function read_object_md(req) {
791791
throw new RpcError('UNAUTHORIZED', 'requesting account is not authorized to read the object');
792792
}
793793

794-
check_md_conditions(md_conditions, obj);
794+
http_utils.check_md_conditions(md_conditions, obj);
795795
const info = get_object_info(obj, { role: req.role });
796796
_check_encryption_permissions(obj.encryption, encryption);
797797

@@ -1593,51 +1593,6 @@ function check_object_mode(req, obj, rpc_code) {
15931593
return obj;
15941594
}
15951595

1596-
function check_md_conditions(conditions, obj) {
1597-
if (!conditions) return;
1598-
if (!conditions.if_match_etag &&
1599-
!conditions.if_none_match_etag &&
1600-
!conditions.if_modified_since &&
1601-
!conditions.if_unmodified_since) return;
1602-
1603-
const data = obj ? {
1604-
etag: obj.etag,
1605-
last_modified: obj.create_time ?
1606-
obj.create_time.getTime() : obj._id.getTimestamp().getTime(),
1607-
} : {
1608-
etag: '',
1609-
last_modified: 0,
1610-
};
1611-
1612-
// See http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectHEAD.html#req-header-consideration-1
1613-
// See https://tools.ietf.org/html/rfc7232 (HTTP Conditional Requests)
1614-
let matched = false;
1615-
let unmatched = false;
1616-
1617-
if (conditions.if_match_etag) {
1618-
if (!(obj && http_utils.match_etag(conditions.if_match_etag, data.etag))) {
1619-
throw new RpcError('IF_MATCH_ETAG', 'check_md_conditions failed', data);
1620-
}
1621-
matched = true;
1622-
}
1623-
if (conditions.if_none_match_etag) {
1624-
if (obj && http_utils.match_etag(conditions.if_none_match_etag, data.etag)) {
1625-
throw new RpcError('IF_NONE_MATCH_ETAG', 'check_md_conditions failed', data);
1626-
}
1627-
unmatched = true;
1628-
}
1629-
if (conditions.if_modified_since) {
1630-
if (!unmatched && (!obj || conditions.if_modified_since > data.last_modified)) {
1631-
throw new RpcError('IF_MODIFIED_SINCE', 'check_md_conditions failed', data);
1632-
}
1633-
}
1634-
if (conditions.if_unmodified_since) {
1635-
if (!matched && (!obj || conditions.if_unmodified_since < data.last_modified)) {
1636-
throw new RpcError('IF_UNMODIFIED_SINCE', 'check_md_conditions failed', data);
1637-
}
1638-
}
1639-
}
1640-
16411596
/**
16421597
* Return the etag ("Entity tag") for the given entity.
16431598
* Entity can be ObjectMD or ObjectMultipart or an updates for one of those.
@@ -1736,8 +1691,8 @@ async function _put_object_handle_latest({ req, put_obj, set_updates, unset_upda
17361691

17371692
if (bucket_versioning === 'DISABLED') {
17381693
const obj = await MDStore.instance().find_object_null_version(req.bucket._id, put_obj.key);
1694+
http_utils.check_md_conditions(req.rpc_params.md_conditions, obj);
17391695
if (obj) {
1740-
check_md_conditions(req.rpc_params.md_conditions, obj);
17411696
// 2, 3, 6, 7
17421697
await MDStore.instance().complete_object_upload_latest_mark_remove_current_and_delete({
17431698
unmark_obj: obj,
@@ -1754,8 +1709,8 @@ async function _put_object_handle_latest({ req, put_obj, set_updates, unset_upda
17541709

17551710
if (bucket_versioning === 'ENABLED') {
17561711
const obj = await MDStore.instance().find_object_latest(req.bucket._id, put_obj.key);
1712+
http_utils.check_md_conditions(req.rpc_params.md_conditions, obj);
17571713
if (obj) {
1758-
check_md_conditions(req.rpc_params.md_conditions, obj);
17591714
// 3, 6
17601715
await MDStore.instance().complete_object_upload_latest_mark_remove_current({
17611716
unmark_obj: obj,
@@ -1773,12 +1728,11 @@ async function _put_object_handle_latest({ req, put_obj, set_updates, unset_upda
17731728
if (bucket_versioning === 'SUSPENDED') {
17741729
const obj = await MDStore.instance().find_object_null_version(req.bucket._id, put_obj.key);
17751730
if (obj) {
1776-
check_md_conditions(req.rpc_params.md_conditions, obj);
17771731
// 2, 3, 6, 7
17781732
if (obj.version_past) {
17791733
const latest_obj = await MDStore.instance().find_object_latest(req.bucket._id, put_obj.key);
1734+
http_utils.check_md_conditions(req.rpc_params.md_conditions, latest_obj);
17801735
if (latest_obj) {
1781-
check_md_conditions(req.rpc_params.md_conditions, latest_obj);
17821736
// 2, 3, 6, 7
17831737
await MDStore.instance().complete_object_upload_latest_mark_remove_current_and_delete({
17841738
delete_obj: obj,
@@ -1792,6 +1746,7 @@ async function _put_object_handle_latest({ req, put_obj, set_updates, unset_upda
17921746
await MDStore.instance().update_object_by_id(put_obj._id, set_updates, unset_updates);
17931747
}
17941748
} else {
1749+
http_utils.check_md_conditions(req.rpc_params.md_conditions, obj);
17951750
await MDStore.instance().complete_object_upload_latest_mark_remove_current_and_delete({
17961751
unmark_obj: obj,
17971752
put_obj,
@@ -1801,8 +1756,8 @@ async function _put_object_handle_latest({ req, put_obj, set_updates, unset_upda
18011756
}
18021757
} else {
18031758
const latest_obj = await MDStore.instance().find_object_latest(req.bucket._id, put_obj.key);
1759+
http_utils.check_md_conditions(req.rpc_params.md_conditions, latest_obj);
18041760
if (latest_obj) {
1805-
check_md_conditions(req.rpc_params.md_conditions, latest_obj);
18061761
// 3, 6
18071762
await MDStore.instance().complete_object_upload_latest_mark_remove_current({
18081763
unmark_obj: latest_obj,
@@ -1828,10 +1783,10 @@ async function _delete_object_version(req) {
18281783

18291784
if (bucket_versioning === 'DISABLED') {
18301785
const obj = version_id === 'null' && await MDStore.instance().find_object_or_upload_null_version(req.bucket._id, req.rpc_params.key);
1786+
http_utils.check_md_conditions(req.rpc_params.md_conditions, obj);
18311787

18321788
if (!obj) return { reply: {} };
18331789
if (obj.delete_marker) dbg.error('versioning disabled bucket null objects should not have delete_markers', obj);
1834-
check_md_conditions(req.rpc_params.md_conditions, obj);
18351790
// 2, 3, 8
18361791
await MDStore.instance().remove_object_and_unset_latest(obj);
18371792
return { obj, reply: _get_delete_obj_reply(obj) };
@@ -1841,6 +1796,7 @@ async function _delete_object_version(req) {
18411796
const obj = version_id === 'null' ?
18421797
await MDStore.instance().find_object_or_upload_null_version(req.bucket._id, req.rpc_params.key) :
18431798
await MDStore.instance().find_object_by_version(req.bucket._id, req.rpc_params.key, version_seq);
1799+
http_utils.check_md_conditions(req.rpc_params.md_conditions, obj);
18441800
if (!obj) return { reply: {} };
18451801

18461802
if (config.WORM_ENABLED && obj.lock_settings) {
@@ -1863,7 +1819,6 @@ async function _delete_object_version(req) {
18631819
}
18641820
}
18651821
}
1866-
check_md_conditions(req.rpc_params.md_conditions, obj);
18671822
if (obj.version_past) {
18681823
// 2, 8
18691824
await MDStore.instance().delete_object_by_id(obj._id);
@@ -1873,7 +1828,6 @@ async function _delete_object_version(req) {
18731828
// we need to find the previous and make it the new latest
18741829
const prev_version = await MDStore.instance().find_object_prev_version(req.bucket._id, req.rpc_params.key);
18751830
if (prev_version) {
1876-
check_md_conditions(req.rpc_params.md_conditions, prev_version);
18771831
// 2, 3, 4, 8
18781832
await MDStore.instance().remove_object_move_latest(obj, prev_version);
18791833
return { obj, reply: _get_delete_obj_reply(obj) };
@@ -1891,8 +1845,8 @@ async function _delete_object_only_key(req) {
18911845

18921846
if (bucket_versioning === 'DISABLED') {
18931847
const obj = await MDStore.instance().find_object_latest(req.bucket._id, req.rpc_params.key);
1848+
http_utils.check_md_conditions(req.rpc_params.md_conditions, obj);
18941849
if (!obj) return { reply: {} };
1895-
check_md_conditions(req.rpc_params.md_conditions, obj);
18961850
if (obj.delete_marker) dbg.error('versioning disabled bucket null objects should not have delete_markers', obj);
18971851
// 2, 3, 8
18981852
await MDStore.instance().remove_object_and_unset_latest(obj);
@@ -1901,8 +1855,8 @@ async function _delete_object_only_key(req) {
19011855

19021856
if (bucket_versioning === 'ENABLED') {
19031857
const obj = await MDStore.instance().find_object_latest(req.bucket._id, req.rpc_params.key);
1858+
http_utils.check_md_conditions(req.rpc_params.md_conditions, obj);
19041859
if (obj) {
1905-
check_md_conditions(req.rpc_params.md_conditions, obj);
19061860
// 3, 5
19071861
const delete_marker = await MDStore.instance().insert_object_delete_marker_move_latest(
19081862
obj, /* version_enabled: */ true);
@@ -1922,12 +1876,11 @@ async function _delete_object_only_key(req) {
19221876
if (bucket_versioning === 'SUSPENDED') {
19231877
const obj = await MDStore.instance().find_object_null_version(req.bucket._id, req.rpc_params.key);
19241878
if (obj) {
1925-
check_md_conditions(req.rpc_params.md_conditions, obj);
19261879
if (obj.version_past) {
19271880
const latest_obj = await MDStore.instance().find_object_latest(req.bucket._id, req.rpc_params.key);
19281881
if (latest_obj) {
1929-
check_md_conditions(req.rpc_params.md_conditions, latest_obj);
19301882
// 3, 5
1883+
http_utils.check_md_conditions(req.rpc_params.md_conditions, latest_obj);
19311884
const delete_marker = await MDStore.instance().insert_object_delete_marker_move_latest_with_delete(obj, latest_obj);
19321885
return { obj, reply: _get_delete_obj_reply(obj, delete_marker) };
19331886
} else {
@@ -1936,13 +1889,14 @@ async function _delete_object_only_key(req) {
19361889
}
19371890
} else {
19381891
// 2, 3, 5
1892+
http_utils.check_md_conditions(req.rpc_params.md_conditions, obj);
19391893
const delete_marker = await MDStore.instance().insert_object_delete_marker_move_latest_with_delete(obj);
19401894
return { obj, reply: _get_delete_obj_reply(obj, delete_marker) };
19411895
}
19421896
} else {
19431897
const latest_obj = await MDStore.instance().find_object_latest(req.bucket._id, req.rpc_params.key);
1898+
http_utils.check_md_conditions(req.rpc_params.md_conditions, latest_obj);
19441899
if (latest_obj) {
1945-
check_md_conditions(req.rpc_params.md_conditions, latest_obj);
19461900
// 3, 5
19471901
const delete_marker = await MDStore.instance().insert_object_delete_marker_move_latest(latest_obj);
19481902
return { reply: _get_delete_obj_reply(null, delete_marker) };

‎src/test/system_tests/ceph_s3_tests/s3-tests-lists/nsfs_s3_tests_pending_list.txt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,6 @@ s3tests_boto3/functional/test_s3.py::test_multi_object_delete_key_limit
116116
s3tests_boto3/functional/test_s3.py::test_multi_objectv2_delete_key_limit
117117
s3tests_boto3/functional/test_s3.py::test_object_write_check_etag
118118
s3tests_boto3/functional/test_s3.py::test_object_metadata_replaced_on_put
119-
s3tests_boto3/functional/test_s3.py::test_get_object_ifmatch_failed
120-
s3tests_boto3/functional/test_s3.py::test_get_object_ifnonematch_good
121-
s3tests_boto3/functional/test_s3.py::test_get_object_ifmodifiedsince_failed
122-
s3tests_boto3/functional/test_s3.py::test_get_object_ifunmodifiedsince_good
123119
s3tests_boto3/functional/test_s3.py::test_put_object_ifmatch_failed
124120
s3tests_boto3/functional/test_s3.py::test_put_object_ifnonmatch_failed
125121
s3tests_boto3/functional/test_s3.py::test_put_object_ifnonmatch_overwrite_existed_failed

‎src/test/system_tests/ceph_s3_tests/s3-tests-lists/s3_tests_black_list.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ s3tests_boto3/functional/test_s3.py::test_post_object_missing_content_length_arg
123123
s3tests_boto3/functional/test_s3.py::test_post_object_invalid_content_length_argument
124124
s3tests_boto3/functional/test_s3.py::test_post_object_upload_size_below_minimum
125125
s3tests_boto3/functional/test_s3.py::test_post_object_empty_conditions
126-
s3tests_boto3/functional/test_s3.py::test_put_object_ifmatch_nonexisted_failed
127126
s3tests_boto3/functional/test_s3.py::test_object_raw_get_bucket_gone
128127
s3tests_boto3/functional/test_s3.py::test_object_raw_get
129128
s3tests_boto3/functional/test_s3.py::test_object_delete_key_bucket_gone

‎src/test/system_tests/ceph_s3_tests/s3-tests-lists/s3_tests_pending_list.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,6 @@ s3tests/functional/test_headers.py::test_object_create_bad_authorization_invalid
129129
s3tests/functional/test_headers.py::test_object_create_bad_date_none_aws2
130130
s3tests/functional/test_headers.py::test_bucket_create_bad_authorization_invalid_aws2
131131
s3tests/functional/test_headers.py::test_bucket_create_bad_date_none_aws2
132-
s3tests_boto3/functional/test_s3.py::test_get_object_ifnonematch_good
133-
s3tests_boto3/functional/test_s3.py::test_get_object_ifmodifiedsince_failed
134132
s3tests_boto3/functional/test_s3.py::test_post_object_wrong_bucket
135133
s3tests_boto3/functional/test_s3.py::test_cors_presigned_put_object_tenant
136134
s3tests_boto3/functional/test_s3.py::test_object_presigned_put_object_with_acl_tenant

0 commit comments

Comments
 (0)