Skip to content

Add support for "Server" and "x-noobaa-available-storage-classes" headers #8255

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

Conversation

tangledbytes
Copy link
Member

@tangledbytes tangledbytes commented Aug 5, 2024

Explain the changes

This PR adds support for:

  1. Server HTTP header whose value is constant and is sent as part of every response.
  2. x-noobaa-available-storage-classes which is sent as part of HeadBucket response. The value could be any combination of storage classes GLACIER and STANDARD based on NooBaa config.

image
image

Issues: Fixed #xxx / Gap #xxx

Fixed #8229

  • Doc added/updated
  • Tests added

@tangledbytes tangledbytes force-pushed the utkarsh/add-support-for-noobaa-specific-headers branch from 3d9db3c to 0484a96 Compare August 6, 2024 09:09
@tangledbytes tangledbytes force-pushed the utkarsh/add-support-for-noobaa-specific-headers branch from 0484a96 to 6dd7dc0 Compare August 9, 2024 10:28
@pull-request-size pull-request-size bot added size/M and removed size/S labels Aug 9, 2024
@tangledbytes tangledbytes force-pushed the utkarsh/add-support-for-noobaa-specific-headers branch from 6dd7dc0 to ef811c9 Compare August 9, 2024 10:29
@tangledbytes tangledbytes requested a review from guymguym August 9, 2024 14:03
@tangledbytes tangledbytes force-pushed the utkarsh/add-support-for-noobaa-specific-headers branch from ef811c9 to b49929d Compare September 2, 2024 10:45
@tangledbytes tangledbytes force-pushed the utkarsh/add-support-for-noobaa-specific-headers branch from b49929d to 0ce7b95 Compare September 4, 2024 09:23
@@ -269,6 +270,8 @@ function create_endpoint_handler(init_request_sdk, virtual_hosts, sts, logger) {

/** @type {EndpointHandler} */
const endpoint_request_handler = (req, res) => {
res.setHeader('Server', `NooBaa/${pkg.version}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but what about the other handlers - sts/iam?
can you add it as function in entpoint_utils and call from all handlers?

// no headers or reply needed
async function head_bucket(req, res) {
const bucket_info = await req.object_sdk.read_bucket({ name: req.params.bucket });
s3_utils.set_response_supported_storage_classes(res, bucket_info.supported_storage_classes || []);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
s3_utils.set_response_supported_storage_classes(res, bucket_info.supported_storage_classes || []);
s3_utils.set_response_supported_storage_classes(res, bucket_info.supported_storage_classes);

*/
function set_response_supported_storage_classes(res, supported_storage_classes = []) {
if (!config.DENY_UPLOAD_TO_STORAGE_CLASS_STANDARD) {
supported_storage_classes.push(STORAGE_CLASS_STANDARD);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so now the storage classes returned by the bucket always do not include STANDARD, which just becomes confusing. I didn't mean that. I only meant that if the bucketspace does not return anything, that we set the default based on the config too, but separating the responsibility like this is too weird. So I think we should just do this instead - have the bucketspace return the list, and if it doesn't return anything, just not set the header.

@tangledbytes tangledbytes force-pushed the utkarsh/add-support-for-noobaa-specific-headers branch from 0ce7b95 to 13bb77c Compare September 5, 2024 09:35
Copy link
Member

@guymguym guymguym left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just missing the call to set_noobaa_server_header to iam handler. approving

@tangledbytes tangledbytes force-pushed the utkarsh/add-support-for-noobaa-specific-headers branch from 13bb77c to 87ce31e Compare September 5, 2024 11:19
…ders

Signed-off-by: Utkarsh Srivastava <[email protected]>

make supported storage classes bucketspace specific

Signed-off-by: Utkarsh Srivastava <[email protected]>

add server header to IAM handler

Signed-off-by: Utkarsh Srivastava <[email protected]>
@tangledbytes tangledbytes force-pushed the utkarsh/add-support-for-noobaa-specific-headers branch from 87ce31e to 4e7790f Compare September 5, 2024 11:50
@tangledbytes tangledbytes merged commit df0c64a into noobaa:master Sep 5, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

S3 head-bucket should return a header with the service identifier (config option)
2 participants