Improvement/arsn 423 bump dependencies #2266

benzekrimaha · 2024-10-21T15:25:22Z

bert-e · 2024-10-21T15:25:26Z

Hello benzekrimaha,

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.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2024-10-21T15:26:18Z

Conflict

There is a conflict between your branch improvement/ARSN-423-bump-dependencies and the
destination branch development/8.2.

Please resolve the conflict on the feature branch (improvement/ARSN-423-bump-dependencies).

git fetch && \
git checkout origin/improvement/ARSN-423-bump-dependencies && \
git merge origin/development/8.2

Resolve merge conflicts and commit

git push origin HEAD:improvement/ARSN-423-bump-dependencies

bert-e · 2024-10-21T15:34:52Z

Waiting for approval

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

the author
2 peers

codecov · 2024-10-21T15:41:08Z

Codecov Report

Attention: Patch coverage is 69.96700% with 91 lines in your changes missing coverage. Please review.

Project coverage is 66.42%. Comparing base (81ee00a) to head (44ed300).

Files with missing lines	Patch %	Lines
lib/policyEvaluator/RequestContext.ts	55.76%	23 Missing ⚠️
lib/patches/locationConstraints.ts	81.81%	10 Missing ⚠️
lib/auth/backends/ChainBackend.ts	27.27%	8 Missing ⚠️
lib/network/kmsAWS/Client.ts	71.42%	8 Missing ⚠️
lib/storage/metadata/mongoclient/LogConsumer.js	12.50%	7 Missing ⚠️
lib/s3routes/routesUtils.ts	0.00%	5 Missing ⚠️
lib/metrics/RedisClient.ts	0.00%	4 Missing ⚠️
lib/s3middleware/azureHelpers/mpuUtils.ts	0.00%	4 Missing ⚠️
lib/auth/auth.ts	57.14%	3 Missing ⚠️
lib/clustering/ClusterRPC.ts	0.00%	3 Missing ⚠️
... and 12 more

Additional details and impacted files

@@                 Coverage Diff                 @@
##           development/8.2    #2266      +/-   ##
===================================================
+ Coverage            66.28%   66.42%   +0.14%     
===================================================
  Files                  215      215              
  Lines                17327    17342      +15     
  Branches              3526     3584      +58     
===================================================
+ Hits                 11485    11520      +35     
+ Misses                5827     5816      -11     
+ Partials                15        6       -9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lib/s3middleware/processMpuParts.ts

lib/policyEvaluator/utils/wildcards.ts

williamlardier · 2024-10-22T07:46:25Z

lib/policyEvaluator/utils/conditions.ts

@@ -279,9 +277,9 @@ export function convertConditionOperator(operator: string): boolean {
            } else {
                return policyValRegex(key);
            }
+            return true;


I think we should return undefined here, because if we reach this stage, it means the "key" was an array and a prefix that is unknown...

tests/unit/algos/list/delimiterMaster.spec.ts

tests/functional/clustering/ClusterRPC.spec.js

lib/models/ObjectMDArchive.ts

lib/models/ObjectMDAmzRestore.ts

lib/models/LifecycleConfiguration.ts

lib/algos/list/delimiter.ts

.github/workflows/tests.yaml

bert-e · 2024-11-19T19:23:29Z

Waiting for approval

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

the author
2 peers

benzekrimaha · 2024-11-19T19:24:48Z

/approve

benzekrimaha · 2024-11-19T19:24:52Z

/wait

bert-e · 2024-11-19T19:24:56Z

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

package.json

williamlardier · 2024-12-02T09:53:56Z

lib/storage/metadata/mongoclient/LogConsumer.js

-                    { error: err });
-                return done(err);
-            }
+            useUnifiedTopology: true,


This shouldn't be needed, I guess this one is deprecated and at least enabled by default, can we check?

reopening because it's still there

(see mongodb/node-mongodb-native#3792)

williamlardier · 2024-12-02T09:57:07Z

lib/s3routes/routesUtils.ts

+            `<Code>${error.message}</Code>`,
+            `<Message>${error.description}</Message>`,
        );


I believe this was changed already in 8.1, you can rebase your branch

The branch has been rebased

lib/network/rpc/sio-stream.ts

tests/unit/policyEvaluator/RequestContext.spec.js

.github/workflows/tests.yaml

francoisferrand · 2024-12-18T09:30:58Z

README.md

+![Public Build Status][badgepub]
+![Private Build Status][badgepriv]
+


+1 for adding badges, but these badges do not seem relevant:

[badgepub]: https://circleci.com/gh/scality/Arsenal.svg?style=svg [badgepriv]: http://ci.ironmann.io/gh/scality/Arsenal.svg?style=svg&circle-token=c3d2570682cba6763a97ea0bc87521941413d75c

Either do not add the badges, or actually fix them, based on gha and codecov.
Not sure what the "private build status" is.

tsconfig.json

lib/network/kmip/Client.ts

README.md

Issue: ARSN-423

In this commit the code has been updated to match the new uuid require based on the documentation here : https://www.npmjs.com/package/uuid timestamp format has been updated as well from new Timestamp(1, 1651144629) to new Timestamp({ t: 1651144629 , i: 1 }), the remaining changes are lint fixups ( please note that the indentation has been fixed as well automatically using lint --fix) Issue: ARSN-423

In this commit we fix the tests that were failing after the dependencies bumps. Issue: ARSN-423

Issue: ARSN-423

williamlardier · 2025-01-07T14:55:48Z

lib/storage/metadata/mongoclient/LogConsumer.js

-                    { error: err });
-                return done(err);
-            }
+            useUnifiedTopology: true,


reopening because it's still there

(see mongodb/node-mongodb-native#3792)

williamlardier · 2025-01-07T14:56:57Z

lib/storage/metadata/mongoclient/MongoClientInterface.js

@@ -477,6 +479,8 @@ class MongoClientInterface {
        const m = this.getCollection(METASTORE);
        m.findOneAndDelete({
            _id: bucketName,
+        } , {
+            includeResultMetadata: true


(same for others)

Suggested change

includeResultMetadata: true

includeResultMetadata: true,

williamlardier · 2025-01-07T14:57:09Z

lib/storage/metadata/mongoclient/MongoClientInterface.js

                upsert: true,
-            }).then(res => {
+            },).then(res => {


Not sure the comma is expected here

williamlardier · 2025-01-07T14:57:53Z

lib/policyEvaluator/requestUtils.ts

@@ -18,8 +18,7 @@ export function getClientIp(request: IncomingMessage, s3config?: S3Config): stri
    const requestConfig = s3config?.requests;
    const remoteAddress = request.socket.remoteAddress;
    // TODO What to do if clientIp === undefined ?
-    const clientIp = (requestConfig ? remoteAddress : request.headers['x-forwarded-for'] ||
-        remoteAddress)?.toString() ?? '';
+    const clientIp = remoteAddress?.toString() ?? '';


Should be already in 8.1 and 8.2, maybe the PR branch is not up to date

See https://github.com/scality/Arsenal/blob/development/8.2/lib/policyEvaluator/requestUtils.ts#L20****

williamlardier · 2025-01-07T15:01:50Z

package.json

    "lint_md": "mdlint $(git ls-files '*.md')",
    "lint_yml": "yamllint $(git ls-files '*.yml')",
-    "test": "jest tests/unit",
+    "test": "export NODE_OPTIONS=\"--tls-max-v1.2\" && jest tests/unit --detectOpenHandles",


Re-opening because I see it's still there as well, and we should support TLS 1.3 today... if we don't support it then we should create a ticket to work on before merging this PR

scality deleted a comment from bert-e Oct 21, 2024

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch from 57632be to 7cfe8b2 Compare October 21, 2024 15:34

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch 3 times, most recently from 75aab28 to 751140a Compare October 22, 2024 07:18

benzekrimaha marked this pull request as ready for review October 22, 2024 07:18

williamlardier reviewed Oct 22, 2024

View reviewed changes

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch 2 times, most recently from 07d79a7 to cb09de6 Compare October 22, 2024 08:23

KillianG reviewed Nov 18, 2024

View reviewed changes

.github/workflows/tests.yaml Outdated Show resolved Hide resolved

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch from 50b1389 to c175969 Compare November 19, 2024 18:20

scality deleted a comment from bert-e Nov 19, 2024

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch from b67e160 to 9cc8a28 Compare November 19, 2024 19:14

scality deleted a comment from bert-e Nov 19, 2024

KillianG reviewed Nov 20, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch from 19e6264 to 9eafa33 Compare November 28, 2024 15:46

benzekrimaha requested review from KillianG and williamlardier November 28, 2024 15:54

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch from 9eafa33 to ba7ade2 Compare November 28, 2024 16:07

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch 2 times, most recently from e4931a8 to 7e142a1 Compare November 28, 2024 18:46

williamlardier reviewed Dec 2, 2024

View reviewed changes

KillianG approved these changes Dec 2, 2024

View reviewed changes

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch from 7e142a1 to e293f89 Compare December 6, 2024 11:14

francoisferrand reviewed Dec 18, 2024

View reviewed changes

.github/workflows/tests.yaml Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 18, 2024

View reviewed changes

tsconfig.json Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 18, 2024

View reviewed changes

lib/network/kmip/Client.ts Outdated Show resolved Hide resolved

francoisferrand reviewed Dec 23, 2024

View reviewed changes

README.md Show resolved Hide resolved

benzekrimaha and others added 8 commits January 2, 2025 11:36

Bump dependencies

9fbe5ee

Issue: ARSN-423

Adapt config post bump eslint to v9

b88a9e3

Issue: ARSN-423

Tests fixups after CI failures due to bumps

3f7c66c

In this commit we fix the tests that were failing after the dependencies bumps. Issue: ARSN-423

Md lint fixup

2ff121d

Issue: ARSN-423

Bump node version in gha

3430234

Issue: ARSN-423

Post reviews fixups

f79ffcf

bump mongodb and adapt code

a04735b

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch 5 times, most recently from ad32261 to 552b601 Compare January 7, 2025 10:54

fixups post reviews

44ed300

benzekrimaha force-pushed the improvement/ARSN-423-bump-dependencies branch from 3096ffe to 44ed300 Compare January 7, 2025 10:56

williamlardier reviewed Jan 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvement/arsn 423 bump dependencies #2266

Improvement/arsn 423 bump dependencies #2266

benzekrimaha commented Oct 21, 2024 •

edited

Loading

bert-e commented Oct 21, 2024

bert-e commented Oct 21, 2024

bert-e commented Oct 21, 2024

codecov bot commented Oct 21, 2024 •

edited

Loading

williamlardier Oct 22, 2024 •

edited

Loading

bert-e commented Nov 19, 2024

benzekrimaha commented Nov 19, 2024

benzekrimaha commented Nov 19, 2024

bert-e commented Nov 19, 2024

williamlardier Dec 2, 2024

williamlardier Jan 7, 2025

williamlardier Dec 2, 2024

benzekrimaha Jan 7, 2025

francoisferrand Dec 18, 2024

williamlardier Jan 7, 2025

williamlardier Jan 7, 2025

williamlardier Jan 7, 2025

williamlardier Jan 7, 2025

williamlardier Jan 7, 2025

williamlardier Jan 7, 2025 •

edited

Loading

		![Public Build Status][badgepub]
		![Private Build Status][badgepriv]

Improvement/arsn 423 bump dependencies #2266

Are you sure you want to change the base?

Improvement/arsn 423 bump dependencies #2266

Conversation

benzekrimaha commented Oct 21, 2024 • edited Loading

bert-e commented Oct 21, 2024

Hello benzekrimaha,

bert-e commented Oct 21, 2024

Conflict

bert-e commented Oct 21, 2024

Waiting for approval

codecov bot commented Oct 21, 2024 • edited Loading

Codecov Report

williamlardier Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

bert-e commented Nov 19, 2024

Waiting for approval

benzekrimaha commented Nov 19, 2024

benzekrimaha commented Nov 19, 2024

bert-e commented Nov 19, 2024

Waiting for approval

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williamlardier Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

benzekrimaha commented Oct 21, 2024 •

edited

Loading

codecov bot commented Oct 21, 2024 •

edited

Loading

williamlardier Oct 22, 2024 •

edited

Loading

williamlardier Jan 7, 2025 •

edited

Loading