CLDSRV-473: fix cors issues in getVeeamFile #5464

hervedombya · 2023-11-30T16:57:23Z

Pull request template

Description

Motivation and context

Why is this change required? What problem does it solve?

Related issues

Please use the following link syntaxes #600 to reference issues in the
current repository

Checklist

Add tests to cover the changes

New tests added or existing tests modified to cover all changes

Code conforms with the style guide

Sign your work

In order to contribute to the project, you must sign your work
https://github.com/scality/Guidelines/blob/master/CONTRIBUTING.md#sign-your-work

Thank you again for contributing! We will try to test and integrate the change
as soon as we can.

bert-e · 2023-11-30T16:57:27Z

Hello hervedombya,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2023-11-30T16:57:32Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-473 contains:

8.8.6

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

8.8.7

Please check the Fix Version/s of CLDSRV-473, or the target
branch of this pull request.

bert-e · 2023-12-06T17:08:04Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-473 contains:

8.8.6
8.8.7
8.8.8

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

8.8.8

Please check the Fix Version/s of CLDSRV-473, or the target
branch of this pull request.

Put the review in the wrong PR.

williamlardier · 2024-01-08T10:46:33Z

lib/routes/veeam/get.js

@@ -18,7 +18,7 @@ function getVeeamFile(request, response, bucketMd, log) {
        return responseXMLBody(errors.NoSuchBucket, null, response, log);
    }
    if ('tagging' in request.query) {
-        return respondWithData(request, response, log, request.bucketName,
+        return respondWithData(request, response, log, bucketMd,


Context for reviewers: passing the whole bucket object instead of the name only will allow to support proxies by returning the right CORS config, API format is not impacted

Context: We had an error in local env when retrieving the capacity xml file, so to fix it we sent the bucket object as a parameter in response instead of just the bucket name.

williamlardier · 2024-01-08T10:47:53Z

lib/routes/veeam/head.js

@@ -35,7 +35,7 @@ function headVeeamFile(request, response, bucketMd, log) {
            headless: true,
        });
        const dataBuffer = Buffer.from(buildHeadXML(builder.buildObject(fileToBuild)));
-        return responseContentHeaders(null, {}, getResponseHeader(request, request.bucketName,
+        return responseContentHeaders(null, {}, getResponseHeader(request, data,


Worth adding a test if possible to ensure that the CORS headers are returned, usually we ensure that each bugfix ticket is merged with a test to ensure that we never recreate the issue again

Done in 363afcd

bert-e · 2024-01-08T10:48:11Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-473 contains:

8.8.15
8.8.6
8.8.7
8.8.8

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

8.8.15

Please check the Fix Version/s of CLDSRV-473, or the target
branch of this pull request.

bert-e · 2024-01-10T15:19:57Z

Branches have diverged

This pull request's source branch bugfix/CLDSRV-473-fix-veeam-get-capacity has diverged from
development/8.8 by more than 50 commits.

To avoid any integration risks, please re-synchronize them using one of the
following solutions:

Merge origin/development/8.8 into bugfix/CLDSRV-473-fix-veeam-get-capacity
Rebase bugfix/CLDSRV-473-fix-veeam-get-capacity onto origin/development/8.8

Note: If you choose to rebase, you may have to ask me to rebuild
integration branches using the reset command.

bert-e · 2024-01-10T15:25:16Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-473 contains:

8.8.15
8.8.6
8.8.7
8.8.8

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

8.8.15

Please check the Fix Version/s of CLDSRV-473, or the target
branch of this pull request.

bert-e · 2024-01-26T14:36:03Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-473 contains:

8.8.16
8.8.6
8.8.7
8.8.8

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

8.8.16

Please check the Fix Version/s of CLDSRV-473, or the target
branch of this pull request.

bert-e · 2024-01-26T15:13:01Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

williamlardier

LGTM
You can add a commit as the last one to bump the project before mergning, if you need to release. Note: make sure the targeted branch is indeed what you want to fix this bug.

hervedombya · 2024-01-30T16:32:05Z

/approve

bert-e · 2024-01-30T16:32:15Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

The following options are set: approve

JBWatenbergScality

LGTM

bert-e · 2024-01-31T13:44:26Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/8.8

The following branches will NOT be impacted:

development/7.10
development/7.4
development/7.70
development/8.6
development/8.7

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2024-01-31T13:44:37Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/8.8

The following branches have NOT changed:

development/7.10
development/7.4
development/7.70
development/8.6
development/8.7

Please check the status of the associated issue CLDSRV-473.

Goodbye hervedombya.

anurag4DSB previously approved these changes Dec 6, 2023

View reviewed changes

williamlardier reviewed Jan 8, 2024

View reviewed changes

hervedombya force-pushed the bugfix/CLDSRV-473-fix-veeam-get-capacity branch from 0ab6b13 to 6c7629f Compare January 10, 2024 15:25

hervedombya force-pushed the bugfix/CLDSRV-473-fix-veeam-get-capacity branch 2 times, most recently from 46c6aa2 to f5bc3d8 Compare January 26, 2024 14:55

CLDSRV-473: fix cors issues in getVeeamFile

363afcd

hervedombya force-pushed the bugfix/CLDSRV-473-fix-veeam-get-capacity branch from f5bc3d8 to 363afcd Compare January 26, 2024 14:59

hervedombya changed the title ~~Bugfix/cldsrv 473 fix veeam get capacity~~ CLDSRV-473: fix cors issues in getVeeamFile Jan 26, 2024

hervedombya changed the title ~~CLDSRV-473: fix cors issues in getVeeamFile~~ CLDSRV-473 // fix cors issues in getVeeamFile Jan 26, 2024

hervedombya changed the title ~~CLDSRV-473 // fix cors issues in getVeeamFile~~ CLDSRV-473: fix cors issues in getVeeamFile Jan 26, 2024

williamlardier approved these changes Jan 30, 2024

View reviewed changes

JBWatenbergScality approved these changes Jan 31, 2024

View reviewed changes

bert-e merged commit 363afcd into development/8.8 Jan 31, 2024
20 checks passed

bert-e deleted the bugfix/CLDSRV-473-fix-veeam-get-capacity branch January 31, 2024 13:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLDSRV-473: fix cors issues in getVeeamFile #5464

CLDSRV-473: fix cors issues in getVeeamFile #5464

hervedombya commented Nov 30, 2023

bert-e commented Nov 30, 2023

bert-e commented Nov 30, 2023

bert-e commented Dec 6, 2023

williamlardier Jan 8, 2024

hervedombya Jan 10, 2024

williamlardier Jan 8, 2024

hervedombya Jan 26, 2024

bert-e commented Jan 8, 2024

bert-e commented Jan 10, 2024

bert-e commented Jan 10, 2024

bert-e commented Jan 26, 2024

bert-e commented Jan 26, 2024

williamlardier left a comment

hervedombya commented Jan 30, 2024

bert-e commented Jan 30, 2024

JBWatenbergScality left a comment

bert-e commented Jan 31, 2024

bert-e commented Jan 31, 2024

CLDSRV-473: fix cors issues in getVeeamFile #5464

CLDSRV-473: fix cors issues in getVeeamFile #5464

Conversation

hervedombya commented Nov 30, 2023

Pull request template

Description

Motivation and context

Related issues

Checklist

Add tests to cover the changes

Code conforms with the style guide

Sign your work

bert-e commented Nov 30, 2023

Hello hervedombya,

bert-e commented Nov 30, 2023

Incorrect fix version

bert-e commented Dec 6, 2023

Incorrect fix version

williamlardier Jan 8, 2024

Choose a reason for hiding this comment

hervedombya Jan 10, 2024

Choose a reason for hiding this comment

williamlardier Jan 8, 2024

Choose a reason for hiding this comment

hervedombya Jan 26, 2024

Choose a reason for hiding this comment

bert-e commented Jan 8, 2024

Incorrect fix version

bert-e commented Jan 10, 2024

Branches have diverged

bert-e commented Jan 10, 2024

Incorrect fix version

bert-e commented Jan 26, 2024

Incorrect fix version

bert-e commented Jan 26, 2024

Waiting for approval

williamlardier left a comment

Choose a reason for hiding this comment

hervedombya commented Jan 30, 2024

bert-e commented Jan 30, 2024

Waiting for approval

JBWatenbergScality left a comment

Choose a reason for hiding this comment

bert-e commented Jan 31, 2024

In the queue

bert-e commented Jan 31, 2024