Skip to content

Commit aa6f37c

Browse files
authored
Merge pull request #8729 from jackyalbo/jacky-cors-fixes
Adding upgrade script to bucket CORS - new bucket default will be the same as in 5.18
2 parents ad2cbb6 + a582132 commit aa6f37c

File tree

8 files changed

+107
-16
lines changed

8 files changed

+107
-16
lines changed

config.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,15 +158,16 @@ config.ENDPOINT_HTTP_MAX_REQUESTS_PER_SOCKET = 0; // 0 = no limit
158158
// note that browsers do not really allow origin=* with credentials,
159159
// but we just allow both from our side for simplicity.
160160
config.S3_CORS_ENABLED = true;
161-
config.S3_CORS_ALLOW_ORIGIN = '*';
161+
config.S3_CORS_DEFAULTS_ENABLED = true;
162+
config.S3_CORS_ALLOW_ORIGIN = ['*'];
162163
config.S3_CORS_ALLOW_CREDENTIAL = 'true';
163164
config.S3_CORS_ALLOW_METHODS = [
164165
'GET',
165166
'POST',
166167
'PUT',
167168
'DELETE',
168169
'OPTIONS'
169-
].join(',');
170+
];
170171
config.S3_CORS_ALLOW_HEADERS = [
171172
'Content-Type',
172173
'Content-MD5',
@@ -177,11 +178,11 @@ config.S3_CORS_ALLOW_HEADERS = [
177178
'X-Amz-Content-Sha256',
178179
'amz-sdk-invocation-id',
179180
'amz-sdk-request',
180-
].join(',');
181+
];
181182
config.S3_CORS_EXPOSE_HEADERS = [
182183
'ETag',
183184
'X-Amz-Version-Id'
184-
].join(',');
185+
];
185186
config.STS_CORS_EXPOSE_HEADERS = 'ETag';
186187

187188
config.DENY_UPLOAD_TO_STORAGE_CLASS_STANDARD = false;

src/sdk/bucketspace_fs.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,14 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
350350
force_md5_etag: force_md5_etag,
351351
path: bucket_storage_path,
352352
should_create_underlying_storage: create_uls,
353-
fs_backend: account.nsfs_account_config.fs_backend
353+
fs_backend: account.nsfs_account_config.fs_backend,
354+
cors_configuration_rules: config.S3_CORS_DEFAULTS_ENABLED ? [{
355+
allowed_origins: config.S3_CORS_ALLOW_ORIGIN,
356+
allowed_methods: config.S3_CORS_ALLOW_METHODS,
357+
allowed_headers: config.S3_CORS_ALLOW_HEADERS,
358+
expose_headers: config.S3_CORS_EXPOSE_HEADERS,
359+
360+
}] : undefined,
354361
};
355362
}
356363

src/server/system_services/bucket_server.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ function new_bucket_defaults(name, system_id, tiering_policy_id, owner_account_i
7474
object_lock_configuration: config.WORM_ENABLED ? {
7575
object_lock_enabled: lock_enabled ? 'Enabled' : 'Disabled',
7676
} : undefined,
77+
cors_configuration_rules: config.S3_CORS_DEFAULTS_ENABLED ? [{
78+
allowed_origins: config.S3_CORS_ALLOW_ORIGIN,
79+
allowed_methods: config.S3_CORS_ALLOW_METHODS,
80+
allowed_headers: config.S3_CORS_ALLOW_HEADERS,
81+
expose_headers: config.S3_CORS_EXPOSE_HEADERS,
82+
}] : undefined,
7783
};
7884
}
7985

src/test/system_tests/ceph_s3_tests/run_ceph_nsfs_test_on_test_container.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export CONFIG_DIR=/etc/noobaa.conf.d/
2525
export FS_ROOT_1=/tmp/nsfs_root1/
2626
export FS_ROOT_2=/tmp/nsfs_root2/
2727
export CONFIG_JS_allow_anonymous_access_in_test=true # Needed for allowing anon access for tests using ACL='public-read-write'
28+
export CONFIG_JS_S3_CORS_DEFAULTS_ENABLED=false # Needed for disabling cors defaults for ceph cors test
2829

2930
# ====================================================================================
3031

src/test/system_tests/ceph_s3_tests/run_ceph_test_on_test_container.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export CEPH_TEST_LOGS_DIR=/logs/ceph-test-logs
3838

3939
export CONFIG_JS_OBJECT_SDK_BUCKET_CACHE_EXPIRY_MS=0 # Needed for disabling cache for ceph cors test and maybe some more
4040
export CONFIG_JS_allow_anonymous_access_in_test=true # Needed for allowing anon access for tests using ACL='public-read-write'
41+
export CONFIG_JS_S3_CORS_DEFAULTS_ENABLED=false # Needed for disabling cors defaults for ceph cors test
4142

4243
# ====================================================================================
4344

src/test/unit_tests/test_upgrade_scripts.js

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,38 @@ const { NodeHttpHandler } = require("@smithy/node-http-handler");
1010
const http = require('http');
1111
const system_store = require('../../server/system_services/system_store').get_instance();
1212
const upgrade_bucket_policy = require('../../upgrade/upgrade_scripts/5.15.6/upgrade_bucket_policy');
13+
const upgrade_bucket_cors = require('../../upgrade/upgrade_scripts/5.19.0/upgrade_bucket_cors');
1314
const dbg = require('../../util/debug_module')(__filename);
1415
const assert = require('assert');
1516
const mocha = require('mocha');
1617
const config = require('../../../config');
1718

1819
const BKT = 'test-bucket';
20+
/** @type {S3} */
1921
let s3;
2022

2123
async function _clean_all_bucket_policies() {
22-
for (const bucket of system_store.data.buckets) {
23-
if (bucket.s3_policy) {
24-
await s3.deleteBucketPolicy({ Bucket: bucket.name.unwrap() });
24+
const buckets = system_store.data.buckets.map(bucket => ({
25+
_id: bucket._id,
26+
$unset: { s3_policy: 1 }
27+
}));
28+
await system_store.make_changes({
29+
update: {
30+
buckets
2531
}
26-
}
32+
});
33+
}
34+
35+
async function _clean_all_bucket_cors() {
36+
const buckets = system_store.data.buckets.map(bucket => ({
37+
_id: bucket._id,
38+
$unset: { cors_configuration_rules: 1 }
39+
}));
40+
await system_store.make_changes({
41+
update: {
42+
buckets
43+
}
44+
});
2745
}
2846

2947
mocha.describe('test upgrade scripts', async function() {
@@ -93,6 +111,24 @@ mocha.describe('test upgrade scripts', async function() {
93111
assert.strictEqual(new_policy.Statement[0].Resource[0], old_policy.statement[0].resource[0]);
94112
});
95113

114+
mocha.it('test upgrade bucket cors to version 5.19.0', async function() {
115+
116+
// clean all leftover bucket CORS configurations
117+
await _clean_all_bucket_cors();
118+
119+
await upgrade_bucket_cors.run({ dbg, system_store });
120+
const cors = await s3.getBucketCors({
121+
Bucket: BKT,
122+
});
123+
124+
dbg.log0('cors:', cors);
125+
126+
assert.deepEqual(cors.CORSRules[0].AllowedHeaders, config.S3_CORS_ALLOW_HEADERS);
127+
assert.deepEqual(cors.CORSRules[0].AllowedMethods, config.S3_CORS_ALLOW_METHODS);
128+
assert.deepEqual(cors.CORSRules[0].AllowedOrigins, config.S3_CORS_ALLOW_ORIGIN);
129+
assert.deepEqual(cors.CORSRules[0].ExposeHeaders, config.S3_CORS_EXPOSE_HEADERS);
130+
});
131+
96132
mocha.after(async function() {
97133
await s3.deleteBucket({ Bucket: BKT });
98134
});
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/* Copyright (C) 2023 NooBaa */
2+
"use strict";
3+
4+
const util = require('util');
5+
const config = require('../../../../config.js');
6+
7+
async function run({ dbg, system_store }) {
8+
9+
try {
10+
dbg.log0('Starting bucket CORS upgrade...');
11+
const buckets = system_store.data.buckets
12+
.map(bucket => ({
13+
_id: bucket._id,
14+
cors_configuration_rules: [{
15+
allowed_origins: config.S3_CORS_ALLOW_ORIGIN,
16+
allowed_methods: config.S3_CORS_ALLOW_METHODS,
17+
allowed_headers: config.S3_CORS_ALLOW_HEADERS,
18+
expose_headers: config.S3_CORS_EXPOSE_HEADERS,
19+
}],
20+
}));
21+
22+
if (buckets.length > 0) {
23+
dbg.log0(`Adding default bucket CORS configuration to: ${buckets.map(bucket => util.inspect(bucket)).join(', ')}`);
24+
await system_store.make_changes({ update: { buckets } });
25+
} else {
26+
dbg.log0('Upgrading buckets CORS configuration: no upgrade needed...');
27+
}
28+
29+
} catch (err) {
30+
dbg.error('Got error while upgrading buckets CORS configuration:', err);
31+
throw err;
32+
}
33+
}
34+
35+
36+
module.exports = {
37+
run,
38+
description: 'Update default CORS configuration for all buckets',
39+
};

src/util/http_utils.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -692,10 +692,10 @@ function set_cors_headers_s3(req, res, cors_rules) {
692692
function set_cors_headers_sts(req, res) {
693693
if (config.S3_CORS_ENABLED) {
694694
set_cors_headers(req, res, {
695-
allow_origin: config.S3_CORS_ALLOW_ORIGIN,
695+
allow_origin: config.S3_CORS_ALLOW_ORIGIN[0],
696696
allow_credentials: config.S3_CORS_ALLOW_CREDENTIAL,
697-
allow_methods: config.S3_CORS_ALLOW_METHODS,
698-
allow_headers: config.S3_CORS_ALLOW_HEADERS,
697+
allow_methods: config.S3_CORS_ALLOW_METHODS.join(','),
698+
allow_headers: config.S3_CORS_ALLOW_HEADERS.join(','),
699699
expose_headers: config.STS_CORS_EXPOSE_HEADERS,
700700
});
701701
}
@@ -756,7 +756,7 @@ function http_get(uri, options) {
756756
* @param {number} https_port
757757
* @param {('S3'|'IAM'|'STS'|'METRICS')} server_type
758758
* @param {Object} request_handler
759-
*/
759+
*/
760760
async function start_https_server(https_port, server_type, request_handler, nsfs_config_root) {
761761
const ssl_cert_info = await ssl_utils.get_ssl_cert_info(server_type, nsfs_config_root);
762762
const https_server = await ssl_utils.create_https_server(ssl_cert_info, true, request_handler);
@@ -775,7 +775,7 @@ async function start_https_server(https_port, server_type, request_handler, nsfs
775775
* @param {number} http_port
776776
* @param {('S3'|'IAM'|'STS'|'METRICS')} server_type
777777
* @param {Object} request_handler
778-
*/
778+
*/
779779
async function start_http_server(http_port, server_type, request_handler) {
780780
const http_server = http.createServer(request_handler);
781781
if (http_port > 0) {
@@ -790,7 +790,7 @@ async function start_http_server(http_port, server_type, request_handler) {
790790
* @param {number} port
791791
* @param {http.Server} server
792792
* @param {('S3'|'IAM'|'STS'|'METRICS')} server_type
793-
*/
793+
*/
794794
function listen_port(port, server, server_type) {
795795
return new Promise((resolve, reject) => {
796796
if (server_type !== 'METRICS') {
@@ -810,7 +810,7 @@ function listen_port(port, server, server_type) {
810810
/**
811811
* Setup endpoint socket and server, Setup is not used for non-endpoint servers.
812812
* @param {http.Server} server
813-
*/
813+
*/
814814
function setup_endpoint_server(server) {
815815
// Handle 'Expect' header different than 100-continue to conform with AWS.
816816
// Consider any expect value as if the client is expecting 100-continue.

0 commit comments

Comments
 (0)