-
Notifications
You must be signed in to change notification settings - Fork 372
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
feat(s3control): Add S3 AccessPoint and AccessPointPolicy #1784
feat(s3control): Add S3 AccessPoint and AccessPointPolicy #1784
Conversation
44af070
to
6dd0820
Compare
fyi, we plan to add policy for s3 access point in another PR |
S3 access poit policies and selectors were added. |
@MisterMX I saw that you did several reviews, could you have a look? |
@MisterMX, thx you for the feedback. I implemented the requested changes. CR for
The structure for the access point policy body was copied from |
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.
@kbujanecki-dt thank you very much for your contribution. In order to avoid duplicate code it would be great if you could reuse the implementation from s3
which is already tested.
@kbujanecki-dt this looks good to me now. Thank you very much for your contribution! Can you please squash your changes into a single commit? Then I can merge it. |
Signed-off-by: kbujanecki-dt <[email protected]>
36d36e3
to
c8e9f65
Compare
I squashed commits and pushed. |
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 very much for your contribution @kbujanecki-dt!
Description of your changes
Fixes #1782
I have:
make reviewable test
to ensure this PR is ready for review.How has this code been tested