NC | Adding support of user bucket path #9271
Conversation
WalkthroughAdds optional custom NSFS bucket path support: new config keys, request/header handling, API/schema additions, bucketspace FS validation against an allow-list, CLI/account plumbing, and integration tests covering success, collision and authorization cases. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant S3Endpoint as S3 Put Bucket Endpoint
participant ObjectSDK
participant BucketspaceFS
participant AccountCfg as Account Config
Client->>S3Endpoint: PUT Bucket (Header: x-noobaa-custom-bucket-path: /custom/path)
S3Endpoint->>S3Endpoint: read header -> params.custom_bucket_path
S3Endpoint->>ObjectSDK: create_bucket(name, lock_enabled, custom_bucket_path)
ObjectSDK->>AccountCfg: fetch nsfs_account_config.custom_bucket_path_allowed_list
ObjectSDK->>BucketspaceFS: create_bucket(params)
alt custom path provided
BucketspaceFS->>AccountCfg: determine allowed list (account or default)
alt path matches allowed prefix
BucketspaceFS->>BucketspaceFS: bucket_storage_path = params.custom_bucket_path
BucketspaceFS->>BucketspaceFS: create/init bucket at custom path
BucketspaceFS-->>ObjectSDK: success
else path not allowed
BucketspaceFS-->>ObjectSDK: throw UNAUTHORIZED / AccessDenied
end
else no custom path
BucketspaceFS->>BucketspaceFS: bucket_storage_path = default (new_buckets_path + name)
BucketspaceFS->>BucketspaceFS: create/init bucket at default path
BucketspaceFS-->>ObjectSDK: success
end
ObjectSDK-->>S3Endpoint: success / error
S3Endpoint-->>Client: 200 OK / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/sdk/bucketspace_fs.js (1)
306-314: Critical: Validate and confine filesystem path from header to prevent unauthorized bucket creationThe
user_bucket_pathparameter (from HTTP headers) is accepted without validation, enabling a client to direct bucket creation to arbitrary filesystem paths. The existing validation function only validates the bucket name, not this path.Apply the suggested guard to ensure
user_bucket_pathis absolutely rejected if it's an absolute path (e.g., /etc/passwd) and confined to remain strictly within the intended base directory:- const { name } = params; - const bucket_config_path = this.config_fs.get_bucket_path_by_name(name); - const bucket_storage_path = params.user_bucket_path || - path.join(sdk.requesting_account.nsfs_account_config.new_buckets_path, name); + const { name } = params; + const bucket_config_path = this.config_fs.get_bucket_path_by_name(name); + let bucket_storage_path; + if (params.user_bucket_path) { + if (typeof params.user_bucket_path !== 'string' || params.user_bucket_path.trim() === '') { + throw new RpcError('INVALID_REQUEST', 'user_bucket_path must be a non-empty string'); + } + const resolved = path.resolve(params.user_bucket_path); + if (!path.isAbsolute(resolved)) { + throw new RpcError('INVALID_REQUEST', 'user_bucket_path must be an absolute path'); + } + const base = path.resolve(sdk.requesting_account.nsfs_account_config.new_buckets_path); + // Constrain to the account's bucket root + if (!(resolved === base || resolved.startsWith(base + path.sep))) { + throw new RpcError('FORBIDDEN', 'user_bucket_path must reside under the account new_buckets_path'); + } + bucket_storage_path = resolved; + } else { + bucket_storage_path = path.join( + sdk.requesting_account.nsfs_account_config.new_buckets_path, + name + ); + }
🧹 Nitpick comments (6)
config.js (1)
1019-1020: Header constant looks good; consider default CORS allow-listing itIf browsers will send this header, add it to S3_CORS_ALLOW_HEADERS to pass preflight.
Apply after the new constant:
config.NSFS_USER_BUCKET_PATH_HTTP_HEADER = 'x-nsfs-bucket-path'; + +// Allow the custom header for browser clients (preflight) +if (Array.isArray(config.S3_CORS_ALLOW_HEADERS) && + !config.S3_CORS_ALLOW_HEADERS.includes(config.NSFS_USER_BUCKET_PATH_HTTP_HEADER)) { + config.S3_CORS_ALLOW_HEADERS.push(config.NSFS_USER_BUCKET_PATH_HTTP_HEADER); +}src/api/bucket_api.js (1)
36-36: Tighten schema for user_bucket_pathEnforce non-empty, absolute path (NSFS-only) to fail early on bad input.
- user_bucket_path: { type: 'string' } + user_bucket_path: { + type: 'string', + minLength: 1, + description: 'Absolute filesystem path for underlying bucket directory (NSFS only).', + pattern: '^/' // require absolute POSIX path + }src/endpoint/s3/ops/s3_put_bucket.js (1)
12-13: Normalize header key to lowercase before lookupNode lowercases incoming header keys. If the header name is overridden via ENV, normalize toLowerCase to avoid mismatches. Based on learnings
- const user_bucket_path = req.headers[config.NSFS_USER_BUCKET_PATH_HTTP_HEADER]; - await req.object_sdk.create_bucket({ name: req.params.bucket, lock_enabled: lock_enabled, user_bucket_path: user_bucket_path }); + const header_name = String(config.NSFS_USER_BUCKET_PATH_HTTP_HEADER || '').toLowerCase(); + const user_bucket_path = typeof req.headers[header_name] === 'string' + ? req.headers[header_name].trim() + : undefined; + await req.object_sdk.create_bucket({ + name: req.params.bucket, + lock_enabled, + user_bucket_path + });src/test/integration_tests/nsfs/test_nsfs_integration.js (3)
383-401: Use the config constant for header name and reflect it in the test titleAvoid string drift and keep title accurate.
- mocha.it('create s3 bucket with x-nsfs-bucket-path', async function() { + mocha.it(`create s3 bucket with ${config.NSFS_USER_BUCKET_PATH_HTTP_HEADER}`, async function() { @@ - s3_correct_uid_default_nsr.middlewareStack.add( + s3_correct_uid_default_nsr.middlewareStack.add( (next, context) => args => { - args.request.headers["x-nsfs-bucket-path"] = x_nsfs_bucket_path; + args.request.headers[config.NSFS_USER_BUCKET_PATH_HTTP_HEADER] = x_nsfs_bucket_path; return next(args); }, { step: "finalizeRequest", name: "addCustomHeader", } );
402-425: Fix typo in test name and reuse the config header constantTitle mentions x-mcg-bucket-path; should be the configured header. Reuse the constant to prevent drift.
- mocha.it('create s3 bucket with x-mcg-bucket-path fail as directory exist', async function() { + mocha.it(`create s3 bucket with ${config.NSFS_USER_BUCKET_PATH_HTTP_HEADER} - fail if directory exists`, async function() { @@ - args.request.headers["x-nsfs-bucket-path"] = x_nsfs_bucket_path; + args.request.headers[config.NSFS_USER_BUCKET_PATH_HTTP_HEADER] = x_nsfs_bucket_path; return next(args);
383-401: Add negative tests for path validation (relative/out-of-root)Once validation is added, add cases that assert INVALID_REQUEST for relative paths and FORBIDDEN for paths outside new_buckets_path.
I can draft the additional test cases after you pick the exact errors to assert.
Also applies to: 402-425
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
config.js(1 hunks)src/api/bucket_api.js(1 hunks)src/endpoint/s3/ops/s3_put_bucket.js(1 hunks)src/sdk/bucketspace_fs.js(1 hunks)src/test/integration_tests/nsfs/test_nsfs_integration.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/test/**/*.*
⚙️ CodeRabbit configuration file
src/test/**/*.*: Ensure that the PR includes tests for the changes.
Files:
src/test/integration_tests/nsfs/test_nsfs_integration.js
🧠 Learnings (2)
📚 Learning: 2025-08-08T13:08:38.361Z
Learnt from: naveenpaul1
Repo: noobaa/noobaa-core PR: 9182
File: src/server/system_services/bucket_server.js:1324-1327
Timestamp: 2025-08-08T13:08:38.361Z
Learning: In src/server/system_services/bucket_server.js, the update_all_buckets_default_pool(req) handler expects req.rpc_params.pool_name to be a plain string (not a SensitiveString wrapper), so calling .unwrap() is not needed there.
Applied to files:
src/endpoint/s3/ops/s3_put_bucket.js
📚 Learning: 2025-07-30T10:20:09.402Z
Learnt from: liranmauda
Repo: noobaa/noobaa-core PR: 9174
File: config.js:1044-1046
Timestamp: 2025-07-30T10:20:09.402Z
Learning: The NooBaa config.js file has a built-in environment variable override system via `load_config_env_overrides()` function. Any config property can be overridden using environment variables prefixed with `CONFIG_JS_`. For example, `CONFIG_JS_NC_HEALTH_BUCKETS_COUNT_LIMIT=10000` would override `config.NC_HEALTH_BUCKETS_COUNT_LIMIT`. The system handles type conversion automatically and provides consistent override patterns across all config values.
Applied to files:
config.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Noobaa Image
- GitHub Check: run-jest-unit-tests
- GitHub Check: run-package-lock-validation
jackyalbo
force-pushed
the
jacky-nc-fixes
branch
from
1bf7a19 to
bc8b718
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/endpoint/s3/ops/s3_put_bucket.js (1)
12-13: Consider adding defensive validation at the entry point.Even if downstream validation exists, applying defense-in-depth principles by adding basic checks here would reduce the attack surface and provide clearer error messages:
const user_bucket_path = req.headers[config.NSFS_USER_BUCKET_PATH_HTTP_HEADER]; +if (user_bucket_path !== undefined) { + if (typeof user_bucket_path !== 'string' || user_bucket_path.trim() === '') { + throw new S3Error(S3Error.InvalidArgument); + } + if (user_bucket_path.includes('\0') || user_bucket_path.includes('..')) { + throw new S3Error(S3Error.InvalidArgument); + } +} await req.object_sdk.create_bucket({ name: req.params.bucket, lock_enabled: lock_enabled, user_bucket_path: user_bucket_path });This provides basic sanitization while allowing comprehensive validation downstream.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
config.js(1 hunks)src/api/bucket_api.js(1 hunks)src/endpoint/s3/ops/s3_put_bucket.js(1 hunks)src/sdk/bucketspace_fs.js(1 hunks)src/test/integration_tests/nsfs/test_nsfs_integration.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- config.js
- src/api/bucket_api.js
- src/sdk/bucketspace_fs.js
- src/test/integration_tests/nsfs/test_nsfs_integration.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T13:08:38.361Z
Learnt from: naveenpaul1
Repo: noobaa/noobaa-core PR: 9182
File: src/server/system_services/bucket_server.js:1324-1327
Timestamp: 2025-08-08T13:08:38.361Z
Learning: In src/server/system_services/bucket_server.js, the update_all_buckets_default_pool(req) handler expects req.rpc_params.pool_name to be a plain string (not a SensitiveString wrapper), so calling .unwrap() is not needed there.
Applied to files:
src/endpoint/s3/ops/s3_put_bucket.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: run-jest-unit-tests
- GitHub Check: run-package-lock-validation
- GitHub Check: Build Noobaa Image
src/sdk/bucketspace_fs.js
Outdated
| const bucket_storage_path = params.user_bucket_path || | ||
| path.join(sdk.requesting_account.nsfs_account_config.new_buckets_path, name); |
There was a problem hiding this comment.
First - the term "user_bucket_path" is confusing - the user has a new_buckets_path where buckets gets created by default. So in this new feature we are actually allowing to override that path, which I would name as "custom_bucket_path".
Second - I think we need some minimal protection for the custom path - Otherwise the request might specify very sensitive paths in the host such as /etc or /root. We need to have some whitelist of paths that the custom_bucket_path is required to be a subpath of, this can be a config such as:
config.NSFS_CUSTOM_BUCKET_PATHS_WHITELIST = ['/ibm', '/gpfs']
I am even contemplating if we can restrict the custom paths to always be a subpath of the account's new_buckets_path to allow more flexibility in restricting every account to a specific subtree of the filesystem.
There was a problem hiding this comment.
We decided to go with two levels of allowed_bucket_paths. per system, using config.js:
config.NSFS_CUSTOM_BUCKET_PATH_ALLOWED_LIST = '/ibm1/:/ibm3/'
or as part of the account, nsfs_account_config, for example:
nsfs_account_config: {
uid: 100,
gid: 100,
new_buckets_path: '/gpfs/new_buckets',
nsfs_only: false,
custom_bucket_path_allowed_list: `/gpfs1/:/gpfs2/`,
}
If it exists, the account's custom_bucket_path_allowed_list will override the system's one.
If neither does, we will drop the request with 'access denied'.
@guymguym @madhuthorat for any last feedback
jackyalbo
force-pushed
the
jacky-nc-fixes
branch
from
bc8b718 to
b76db97
Compare
jackyalbo
force-pushed
the
jacky-nc-fixes
branch
3 times, most recently
from
b982863 to
5f51d95
Compare
Signed-off-by: jackyalbo <jacky.albo@gmail.com>
jackyalbo
force-pushed
the
jacky-nc-fixes
branch
from
5f51d95 to
9bbc2a5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cmd/manage_nsfs.js (1)
486-510: fs_backend empty-string deletion semantics are broken; fix confirmedThe issue is verified and accurate. Line 506 uses a truthiness check:
fs_backend: user_input.fs_backend ? String(user_input.fs_backend) : config.NSFS_NC_STORAGE_BACKEND,This prevents empty strings from reaching the normalization at line 534 (
data.nsfs_account_config.fs_backend = data.nsfs_account_config.fs_backend || undefined;), breaking the intended "deletion via empty string" behavior.The bucket version (line 133) correctly uses an explicit
undefinedcheck, allowing empty strings through:fs_backend: user_input.fs_backend === undefined ? config.NSFS_NC_STORAGE_BACKEND : String(user_input.fs_backend),The
custom_bucket_path_allowed_listat line 507 is handled correctly—it assigns directly and lets line 547's normalization handle falsy values.Apply the suggested fix at line 506:
- fs_backend: user_input.fs_backend ? String(user_input.fs_backend) : config.NSFS_NC_STORAGE_BACKEND, + fs_backend: user_input.fs_backend === undefined + ? config.NSFS_NC_STORAGE_BACKEND + : String(user_input.fs_backend),
🧹 Nitpick comments (3)
src/test/integration_tests/nsfs/test_nsfs_integration.js (3)
418-457: Happy-path test for custom bucket path + allowed-list is solidCreating a dedicated account with
custom_bucket_path_allowed_listincludingtmp_fs_rootand then asserting that:
- the bucket create succeeds when the header points under
new_buckets_path, and- the filesystem path at
x_nsfs_bucket_pathexistsgives good end-to-end coverage that the allow-list is honored and the header path is actually used. As an optional cleanup, you could extract the repeated middleware wiring into a small helper to reduce duplication across these tests.
As per coding guidelines, the new behavior is covered by tests under
src/test/**.
459-485: Directory-exists collision test for custom path behaves as expectedThis test reuses the account with an allow-list to ensure:
- creating a bucket with a custom path that already exists returns
BucketAlreadyExists, and- no directory is created at the default new_buckets_path (when the header is present).
That correctly validates collision handling and that the header overrides the default location rather than falling back to it. For robustness and readability, consider using a single consistent pattern for building
x_nsfs_bucket_pathandno_x_nsfs_bucket_path(e.g., viapath.joinor a shared helper) to avoid relying on implicit trailing slashes innew_buckets_path.
486-508: Disallowed-prefix test is correct; comment could be clarifiedThe test correctly checks that:
- using a custom path like
/root/${bucket_name}-custom-paththat is outside the account’scustom_bucket_path_allowed_listreturnsAccessDenied, and- the target path is not created on disk.
The inline comment on Line 489 ("already created in previous test") is misleading, since
/root/...is intentionally not created earlier; cleaning that up would avoid future confusion. You might also align the path construction style here with the earlier tests for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
config.js(1 hunks)src/api/account_api.js(1 hunks)src/api/bucket_api.js(1 hunks)src/api/common_api.js(1 hunks)src/cmd/manage_nsfs.js(2 hunks)src/endpoint/s3/ops/s3_put_bucket.js(1 hunks)src/manage_nsfs/manage_nsfs_constants.js(3 hunks)src/manage_nsfs/manage_nsfs_help_utils.js(2 hunks)src/sdk/accountspace_fs.js(1 hunks)src/sdk/bucketspace_fs.js(1 hunks)src/server/system_services/schemas/nsfs_account_schema.js(2 hunks)src/test/integration_tests/nsfs/test_nsfs_integration.js(3 hunks)src/test/utils/coretest/nc_coretest.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/endpoint/s3/ops/s3_put_bucket.js
- src/api/bucket_api.js
- config.js
- src/sdk/bucketspace_fs.js
🧰 Additional context used
📓 Path-based instructions (1)
src/test/**/*.*
⚙️ CodeRabbit configuration file
src/test/**/*.*: Ensure that the PR includes tests for the changes.
Files:
src/test/utils/coretest/nc_coretest.jssrc/test/integration_tests/nsfs/test_nsfs_integration.js
🧠 Learnings (1)
📚 Learning: 2025-11-13T07:56:23.620Z
Learnt from: shirady
Repo: noobaa/noobaa-core PR: 9281
File: src/server/system_services/account_server.js:1053-1058
Timestamp: 2025-11-13T07:56:23.620Z
Learning: In noobaa-core, account_server.js is only used in containerized deployments, not in NSFS/NC deployments. NSFS/NC deployments have separate account management code in src/manage_nsfs/ directory. Therefore, account_server.js only processes accounts from account_schema.js where owner is an objectid reference, never from nsfs_account_schema.js where owner is a string.
Applied to files:
src/server/system_services/schemas/nsfs_account_schema.jssrc/sdk/accountspace_fs.jssrc/cmd/manage_nsfs.jssrc/test/utils/coretest/nc_coretest.jssrc/api/common_api.js
🧬 Code graph analysis (1)
src/api/common_api.js (16)
src/api/account_api.js (1)
SensitiveString(4-4)src/cmd/manage_nsfs.js (1)
SensitiveString(25-25)src/sdk/accountspace_fs.js (1)
SensitiveString(16-16)src/sdk/bucketspace_fs.js (1)
SensitiveString(28-28)src/test/integration_tests/nsfs/test_nsfs_integration.js (1)
SensitiveString(21-21)src/test/utils/coretest/nc_coretest.js (1)
SensitiveString(10-10)src/server/common_services/auth_server.js (1)
SensitiveString(12-12)src/util/account_util.js (1)
SensitiveString(9-9)src/server/system_services/bucket_server.js (1)
SensitiveString(14-14)src/server/system_services/account_server.js (1)
SensitiveString(15-15)src/server/system_services/schemas/account_schema.js (1)
SensitiveString(4-4)src/server/system_services/schemas/bucket_schema.js (1)
SensitiveString(4-4)src/sdk/config_fs.js (1)
SensitiveString(13-13)src/cmd/nsfs.js (1)
SensitiveString(38-38)src/endpoint/s3/ops/s3_get_bucket_logging.js (1)
SensitiveString(4-4)src/server/system_services/tier_server.js (1)
SensitiveString(19-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: ceph-s3-tests / Ceph S3 Tests
- GitHub Check: mint-nc-tests / Mint NC Tests
- GitHub Check: mint-tests / Mint Tests
- GitHub Check: run-nc-unit-tests / Non Containerized Unit Tests
- GitHub Check: warp-nc-tests / Warp NC Tests
- GitHub Check: warp-tests / Warp Tests
- GitHub Check: ceph-nsfs-s3-tests / NSFS Ceph S3 Tests
- GitHub Check: run-sanity-tests / Sanity Tests
- GitHub Check: run-sanity-ssl-tests / Sanity SSL Tests
- GitHub Check: run-unit-tests / Unit Tests
- GitHub Check: run-unit-tests-postgres / Unit Tests with Postgres
- GitHub Check: run-jest-unit-tests
🔇 Additional comments (11)
src/sdk/accountspace_fs.js (1)
590-613: Propagating custom_bucket_path_allowed_list to new users is consistent and safeCopying
custom_bucket_path_allowed_listfromrequesting_account.nsfs_account_configinto new user defaults aligns with how other NSFS fields are propagated and ensures IAM users respect the same bucket-path policy as their owner.src/api/account_api.js (1)
280-313: Schema extension for custom_bucket_path_allowed_list in update_account_s3_access looks correctAdding
custom_bucket_path_allowed_listtonsfs_account_confighere matches the common_api and nsfs_account_schema definitions and keeps the update path in sync with account creation / manage_nsfs flows.src/test/utils/coretest/nc_coretest.js (1)
327-343: Test helpers wiring custom_bucket_path_allowed_list into manage_nsfs CLI is aligned with runtime changesPropagating
nsfs_account_config.custom_bucket_path_allowed_listinto the add/update account CLI options keeps the NC core tests compatible with the new field and allows exercising the allow-list behavior end‑to‑end.Also applies to: 431-439
src/api/common_api.js (1)
1445-1469: nsfs_account_config schema now includes custom_bucket_path_allowed_list in both variantsAdding
custom_bucket_path_allowed_list: { type: 'string' }to both nsfs_account_config oneOf branches is consistent with the rest of the PR (account_api, nsfs_account_schema, manage_nsfs) and maintains backward compatibility since it’s not required.src/server/system_services/schemas/nsfs_account_schema.js (1)
77-105: NSFS account schema aligned with new custom_bucket_path_allowed_list propertyIncluding
custom_bucket_path_allowed_listin both nsfs_account_config shapes keeps the on‑disk schema compatible with the updated common/account APIs and manage_nsfs tooling; the field is optional, so existing accounts remain valid.src/test/integration_tests/nsfs/test_nsfs_integration.js (3)
2-2: Eslint max-lines override is reasonableRaising
max-linesto 3000 is consistent with the current file size (~2500 lines) and avoids spurious lint failures without behavioral impact.
84-91: New S3 client variable for custom bucket-path tests is scoped correctly
let s3_correct_uid_bucket_path;is in the same scope as the other S3 clients and used only in the new custom-bucket-path tests, so there’s no leakage or shadowing risk.
393-416: Negative test for header without allowed-list looks correctThis test validates that using
config.NSFS_CUSTOM_BUCKET_PATH_HTTP_HEADERwithout configuringcustom_bucket_path_allowed_liston the account yieldsAccessDeniedand does not create the target directory, which matches the intended security behavior.src/manage_nsfs/manage_nsfs_help_utils.js (2)
133-147: Add-account help for--custom_bucket_path_allowed_listis aligned with semanticsThe new flag description clearly conveys that the value is a colon-separated list of allowed custom bucket paths and uses the same option name as the internal wiring (
custom_bucket_path_allowed_list), so users will discover the feature viaaccount add --help.
160-175: Update-account help correctly documents override/unset behaviorThe update flag text explains that
--custom_bucket_path_allowed_listoverrides the existing list and can be unset with'', which matches how UNSETTABLE options are handled in the constants. This keeps CLI behavior and help in sync.src/manage_nsfs/manage_nsfs_constants.js (1)
48-54: CLI wiring forcustom_bucket_path_allowed_listis consistent and completeIncluding
custom_bucket_path_allowed_listin:
VALID_OPTIONS_ACCOUNT.add/.update,OPTION_TYPEas a'string', andUNSETTABLE_OPTIONS_OBJwithCLI_EMPTY_STRINGensures the new flag is accepted for account add/update, typed correctly, and can be unset via
''as documented in the help. This lines up cleanly with the account schema and CLI help text for the feature.Also applies to: 119-127, 193-201
|
@nadavMiz if you could take a look please |
3 participants
Describe the Problem
Adding support for an out-of-S3 header to let the user control where the bucket is supposed to be created.
x-noobaa-custom-bucket-path - will point to where the user wants their new bucket to be saved in the file system.
Explain the Changes
config.NSFS_CUSTOM_BUCKET_PATH_ALLOWED_LIST = '/ibm1/:/ibm3/'
or as part of the account, nsfs_account_config, for example:
nsfs_account_config: {
uid: 100,
gid: 100,
new_buckets_path: '/gpfs/new_buckets',
nsfs_only: false,
custom_bucket_path_allowed_list:
/gpfs1/:/gpfs2/,}
If it exists, the account's custom_bucket_path_allowed_list will override the system's one.
If neither does, we will drop the request with 'access denied'.
Issues: Fixed #xxx / Gap #xxx
Testing Instructions:
custom_bucket_path_allowed_listin config.js and/or in your accountx-noobaa-custom-bucket-pathheader as part of S3 put_bucket request (using account creds)Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.