-
Notifications
You must be signed in to change notification settings - Fork 243
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
CLDSRV-428: put apis updated for implicit deny #5456
CLDSRV-428: put apis updated for implicit deny #5456
Conversation
Hello benzekrimaha,My role is to assist you with the merge of this Status report is not available. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command:
Alternatively, the |
/create_integration_branches |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: $ git fetch
$ git checkout -B w/7.70/improvement/CLDSRV-428-put-apis-impDeny origin/development/7.70
$ git merge origin/improvement/CLDSRV-428-put-apis-impDeny
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/7.70/improvement/CLDSRV-428-put-apis-impDeny The following options are set: create_integration_branches |
2079ee8
to
a8a63de
Compare
ping |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: $ git fetch
$ git checkout -B w/8.6/improvement/CLDSRV-428-put-apis-impDeny origin/development/8.6
$ git merge origin/w/7.70/improvement/CLDSRV-428-put-apis-impDeny
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/8.6/improvement/CLDSRV-428-put-apis-impDeny The following options are set: create_integration_branches |
ping |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
/reset |
Reset completeI have successfully deleted this pull request's integration branches. The following options are set: create_integration_branches |
198f091
to
f7fb413
Compare
In this commit put apis have been updated to check for implicit deny returned by vault and added as a parameter in the request Object. Tests have also been added for the metadataUtils validateBucket function. MetadataUtils functions have been updated to check for implicit deny. The goal is to implement the same authorization logic as AWS, where an implicit deny from IAM and an Allow from the Bucket Policy should allow the request for example. For the delete on the objectPutCopyPart and objectPutPart as we need to deferentiate between the vault request and the external backend once a delete is applied to the request directly as it's unique per API call this value is then added to the request object. here's the link to the design doc for more details: https://github.com/scality/citadel/blob/development/1.0/docs/design/bucket-policies.md?plain=1#L263
f7fb413
to
2596f3f
Compare
ping |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: $ git fetch
$ git checkout -B w/7.70/improvement/CLDSRV-428-put-apis-impDeny origin/development/7.70
$ git merge origin/improvement/CLDSRV-428-put-apis-impDeny
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/7.70/improvement/CLDSRV-428-put-apis-impDeny The following options are set: create_integration_branches |
ping |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: $ git fetch
$ git checkout -B w/8.6/improvement/CLDSRV-428-put-apis-impDeny origin/development/8.6
$ git merge origin/w/7.70/improvement/CLDSRV-428-put-apis-impDeny
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/8.6/improvement/CLDSRV-428-put-apis-impDeny The following options are set: create_integration_branches |
ping |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
@bert-e create_pull_requests |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
Follow integration pull requests if you would like to be notified of The following options are set: create_pull_requests, create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_pull_requests, create_integration_branches |
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, great work.
I have reviewed this PR and integration PRs as well.
Worth adding a commit on top for package.json, bert-e will port forward the separate commit, integration PRs will have conflicts around versions that you can resolve. This way we can release cloudserver.
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the The following options are set: create_pull_requests, create_integration_branches |
ping for version check |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_pull_requests, create_integration_branches |
ConflictThere is a conflict between your branch Please resolve the conflict on the feature branch ( git fetch && \
git checkout origin/improvement/CLDSRV-428-put-apis-impDeny && \
git merge origin/development/7.10 Resolve merge conflicts and commit git push origin HEAD:improvement/CLDSRV-428-put-apis-impDeny The following options are set: create_pull_requests, create_integration_branches |
ConflictA conflict has been raised during the update of Please resolve the conflict on the integration branch ( Here are the steps to resolve this conflict: $ git fetch
$ git checkout w/7.70/improvement/CLDSRV-428-put-apis-impDeny
$ git pull # or "git reset --hard origin/w/7.70/improvement/CLDSRV-428-put-apis-impDeny"
$ git merge origin/development/7.70
$ # <intense conflict resolution>
$ git commit
$ git merge origin/improvement/CLDSRV-428-put-apis-impDeny
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/7.70/improvement/CLDSRV-428-put-apis-impDeny The following options are set: create_pull_requests, create_integration_branches |
ping |
ConflictA conflict has been raised during the update of Please resolve the conflict on the integration branch ( Here are the steps to resolve this conflict: $ git fetch
$ git checkout w/8.6/improvement/CLDSRV-428-put-apis-impDeny
$ git pull # or "git reset --hard origin/w/8.6/improvement/CLDSRV-428-put-apis-impDeny"
$ git merge origin/development/8.6
$ # <intense conflict resolution>
$ git commit
$ git merge origin/w/7.70/improvement/CLDSRV-428-put-apis-impDeny
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/8.6/improvement/CLDSRV-428-put-apis-impDeny The following options are set: create_pull_requests, create_integration_branches |
ping |
ConflictA conflict has been raised during the update of Please resolve the conflict on the integration branch ( Here are the steps to resolve this conflict: $ git fetch
$ git checkout w/8.7/improvement/CLDSRV-428-put-apis-impDeny
$ git pull # or "git reset --hard origin/w/8.7/improvement/CLDSRV-428-put-apis-impDeny"
$ git merge origin/development/8.7
$ # <intense conflict resolution>
$ git commit
$ git merge origin/w/8.6/improvement/CLDSRV-428-put-apis-impDeny
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/8.7/improvement/CLDSRV-428-put-apis-impDeny The following options are set: create_pull_requests, create_integration_branches |
ConflictA conflict has been raised during the update of Please resolve the conflict on the integration branch ( Here are the steps to resolve this conflict: $ git fetch
$ git checkout w/8.8/improvement/CLDSRV-428-put-apis-impDeny
$ git pull # or "git reset --hard origin/w/8.8/improvement/CLDSRV-428-put-apis-impDeny"
$ git merge origin/development/8.8
$ # <intense conflict resolution>
$ git commit
$ git merge origin/w/8.7/improvement/CLDSRV-428-put-apis-impDeny
$ # <intense conflict resolution>
$ git commit
$ git push -u origin w/8.8/improvement/CLDSRV-428-put-apis-impDeny The following options are set: create_pull_requests, create_integration_branches |
ping |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_pull_requests, create_integration_branches |
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 too
/approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve, create_pull_requests, create_integration_branches |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-428. Goodbye benzekrimaha. |
PR opened after closing : #5325 and #5450
Bucket policies are not correctly interpreted, this is part of the following epic to fix that: scality/Arsenal#2181
This PR is aiming to update put apis since object and bucket authorisations are made at API level and need to support implicit denies, ticket linked to this issue here : https://scality.atlassian.net/browse/CLDSRV-428
PRs providing implicit Deny logic to CS for processing in this PR
scality/Arsenal#2181
https://github.com/scality/Vault/pull/2135
#5322
#5420
#5432
Here CI links for zenko tests :
https://github.com/scality/Zenko/actions/runs/7008162132
https://github.com/scality/Zenko/actions/runs/7008182601
https://github.com/scality/Zenko/actions/runs/7008194429
I'll be bumping a new CLDSRV once the reviews done as these are changes that will be tested against Integration