-
Notifications
You must be signed in to change notification settings - Fork 413
fix(api): enforce import_enabled flag for local storage #9417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(api): enforce import_enabled flag for local storage #9417
Conversation
01018eb to
3504a8f
Compare
3504a8f to
15d359c
Compare
nopcoder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you.
nopcoder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By bad, it was approved but not merged.
nopcoder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 2nd time
Closes #8844
Change Description
Background
The
import_enabledflag for local storage is not enforced by the server's API. While the WebUI and lakectl prevent imports when this flag is disabled, a user can bypass the restriction by calling the API directly.Bug Fix
import.enabledflag is set tofalse.ImportStartAPI handler did not check if the underlying storage adapter was configured to support the import operation.ImportStarthandler inpkg/api/controller.gohas been updated. After validating user permissions, it now calls theGetStorageNamespaceInfomethod on the block adapter. It then checks the returnedImportSupportflag and, if it'sfalse, immediately returns a403 Forbiddenerror. This approach is more robust and future-proof.New Feature
Not applicable. This is an enhancement to enforce an existing configuration.
Testing Details
The fix was tested with a new automated test.
TestController_ImportStart_Disabled, was added topkg/api/controller_test.go. It usesviper.Setto override the server configuration to disable local imports, starts a test server, calls the import API, and asserts that the response is403 Forbiddenwith the correct error message.Breaking Change?
No. This change enforces an existing configuration setting as intended and does not break any existing valid functionality.
Additional info
API Response Before:
{"id":"d2doq6vrp4e4ba2rnb50"}API Response After:
{"message":"import is not supported for this storage"}Contact Details
Provided via my GitHub account.